Change in coreboot[master]: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options
Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options
Change-Id: I66a636f554d18e08a209a7cfd6a59cf13a88f2e1 Signed-off-by: Felix Singer felixsinger@posteo.net --- M Documentation/releases/coreboot-4.13-relnotes.md 1 file changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/47409/1
diff --git a/Documentation/releases/coreboot-4.13-relnotes.md b/Documentation/releases/coreboot-4.13-relnotes.md index 8d19067..c8a78bd 100644 --- a/Documentation/releases/coreboot-4.13-relnotes.md +++ b/Documentation/releases/coreboot-4.13-relnotes.md @@ -58,4 +58,25 @@ It still needs changes in assembly, fixed integer to pointer conversions in C, wrappers for blobs, support for running Option ROMs, among other things.
+### Preperations for dropping PCI bus mastering enablement + +The configuration of PCI bus mastering will be dropped in a future release. For +security reasons, bus mastering should be enabled as late as possible. In +coreboot, it's usually not necessary and payloads should only enable it for +devices they use. Since not all payloads enable bus mastering properly yet, +some Kconfig options were added as an intermediate step to give some sort of +"backwards compatibility", which allow enabling or disabling bus mastering by +groups. + +Currently available groups are: + +* Device of class type "system" +* PCI bridges +* Any devices + +For now, "Any devices" is enabled by default to keep the traditional behaviour, +which also includes all other options. This is currently necessary, for instance, +for libpayload based payloads as the drivers don't enable bus mastering for PCI +bridges. + ### Add significant changes here
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 61: Preperations typo: Prep*a*rations
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 61: enablement I don't really like how `enablement` sounds, but can't find something else to use instead (other than rewriting the whole phrase)
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 63: The configuration of PCI bus mastering will be dropped in a future release. I find this to be very unclear. Will setting Bus Master in coreboot be impossible/forbidden?
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 79: libpayload based libpayload-based
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 61: enablement
I don't really like how `enablement` sounds, but can't find something else to use instead (other tha […]
I also don't like it. Maybe "activation"? But I think it's not much better.
"configuration" isn't very specific IMO, like it's not clear what the default value is, but it sounds way better.
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 63: The configuration of PCI bus mastering will be dropped in a future release.
I find this to be very unclear. […]
Not sure, but I think so since coreboot does not need it?
Other opinions?
Hello build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47409
to look at the new patch set (#2).
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options
Change-Id: I66a636f554d18e08a209a7cfd6a59cf13a88f2e1 Signed-off-by: Felix Singer felixsinger@posteo.net --- M Documentation/releases/coreboot-4.13-relnotes.md 1 file changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/47409/2
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 61: Preperations
typo: Prep*a*rations
Done
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 79: libpayload based
libpayload-based
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 61: enablement
I also don't like it. Maybe "activation"? But I think it's not much better. […]
maybe: Preparations to minimize enabling PCI bus mastering in coreboot
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 63: The configuration of PCI bus mastering will be dropped in a future release.
Not sure, but I think so since coreboot does not need it? […]
I'd reword this to feel less absolute, e.g. {minimize, reduce, cut down on} Bus Master usage (instead of dropping it altogether). I'm not sure if eliminating everything will be possible without causing troubles for some people.
[Random brain dump] If I try to achieve "complete" goals (e.g. finish all parts of $complex_task), I will quickly be overwhelmed (oh no I've got to do ALL of this AAAAAAAA) and then fail at getting anything done at all. This is why I prefer more humble, less spectacular goals (e.g. do the first part of $complex_task): as these goals are reasonable to achieve, so I can convince myself to get them done.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 63: The configuration of PCI bus mastering will be dropped in a future release.
I'd reword this to feel less absolute, e.g. […]
I've given examples of cases where we have to enable it on the ML. So no, we can't completely ignore it in coreboot. We should clearly mention that the added Kconfigs are about cases where the enabling is not strictly needed.
Maybe split the whole thing; i.e. remove the first sentence here. And add a deprecation-announcement section like we had in early release notes. Then simply state _there_ that the Kconfigs described _here_ will be dropped. And maybe add:
Exceptional cases that may still need early bus master enabling in the future should get their own per-reason Kconfig option. Ideally before the next release.
Hello build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47409
to look at the new patch set (#3).
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options
Change-Id: I66a636f554d18e08a209a7cfd6a59cf13a88f2e1 Signed-off-by: Felix Singer felixsinger@posteo.net --- M Documentation/releases/coreboot-4.13-relnotes.md 1 file changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/47409/3
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 61: enablement
maybe: Preparations to minimize enabling PCI bus mastering in coreboot
Sounds way better :)
Hello build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47409
to look at the new patch set (#4).
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options
Change-Id: I66a636f554d18e08a209a7cfd6a59cf13a88f2e1 Signed-off-by: Felix Singer felixsinger@posteo.net --- M Documentation/releases/coreboot-4.13-relnotes.md 1 file changed, 32 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/47409/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 4: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 61: Preparations to minimize enabling PCI bus mastering AFAIK, in English, one capitalizes all nouns and verbs in headings, i.e.
### Preparations to Minimize Enabling PCI Bus Mastering in coreboot
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 61: in coreboot Might want to drop this, everything here is "in coreboot".
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 93: here I assume "here" would be the visible text for the link? This is actually one of the don'ts we learned in the '90s when HTML came up. It would be better to repeat (part of) the title, e.g.
see [Preparations to Minimize Enabling PCI Bus Mastering](#...).
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 4: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 61: Preparations to minimize enabling PCI bus mastering
AFAIK, in English, one capitalizes all nouns and verbs in headings, i.e. […]
normally yes, but since it's not done in this file at all, let's be consistent, even when it's consistently wrong then.
PCI Bust Mastering is a name, though, isn't it?
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 67: by "in"?
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 91: PCI bus mastering isn't that a name? PCI Bus Mastering
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 61: in coreboot
Might want to drop this, everything here is "in coreboot".
Agreed
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 61: Preparations to minimize enabling PCI bus mastering
AFAIK, in English, one capitalizes all nouns and verbs in headings, i.e. […]
This wasn't done in the coreboot 4.12 release notes, though. If anything, all headers should be changed at once.
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 93: here
I assume "here" would be the visible text for the link? This is actually one […]
Yes, it would show up as "here". You can make this more OCD-friendly by using a reference-style link:
see [Preparations to minimize enabling PCI bus mastering in coreboot][short-name].
...
[short-name]: #preparations-to-minimize-enabling-pci-bus-mastering-in-coreboot
I did NOT test this.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 4: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 61: Preparations to minimize enabling PCI bus mastering
This wasn't done in the coreboot 4.12 release notes, though. […]
I don't think PCI has anything to do with sculpture ("Bust Mastering") 😄
I usually capitalize "Bus Master" when I write my comments and commit messages, but I'm not 100% certain why I do so.
Hello build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47409
to look at the new patch set (#5).
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options
Change-Id: I66a636f554d18e08a209a7cfd6a59cf13a88f2e1 Signed-off-by: Felix Singer felixsinger@posteo.net --- M Documentation/releases/coreboot-4.13-relnotes.md 1 file changed, 32 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/47409/5
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/4/Documentation/releases/core... PS4, Line 91: PCI bus mastering
isn't that a name? PCI Bus Mastering
Done
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/1/Documentation/releases/core... PS1, Line 63: The configuration of PCI bus mastering will be dropped in a future release.
I've given examples of cases where we have to enable it on the ML. So no, […]
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 5: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 5: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47409/5/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/5/Documentation/releases/core... PS5, Line 72: * Device of class type "system" Actually, this is not a separate "group" in Kconfig, is it?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 5:
Going to submit this before I forget.
Hello build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47409
to look at the new patch set (#6).
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options
Change-Id: I66a636f554d18e08a209a7cfd6a59cf13a88f2e1 Signed-off-by: Felix Singer felixsinger@posteo.net --- M Documentation/releases/coreboot-4.13-relnotes.md 1 file changed, 31 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/47409/6
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47409/5/Documentation/releases/core... File Documentation/releases/coreboot-4.13-relnotes.md:
https://review.coreboot.org/c/coreboot/+/47409/5/Documentation/releases/core... PS5, Line 72: * Device of class type "system"
Actually, this is not a separate "group" in Kconfig, is it?
Removed it
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 6: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 6: Code-Review+2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
Patch Set 7: Code-Review+2
Michael Niewöhner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47409 )
Change subject: doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options ......................................................................
doc/relnotes/4.13: Add note about PCI bus mastering Kconfig options
Change-Id: I66a636f554d18e08a209a7cfd6a59cf13a88f2e1 Signed-off-by: Felix Singer felixsinger@posteo.net Reviewed-on: https://review.coreboot.org/c/coreboot/+/47409 Reviewed-by: Michael Niewöhner foss@mniewoehner.de Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M Documentation/releases/coreboot-4.13-relnotes.md 1 file changed, 31 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, approved Michael Niewöhner: Looks good to me, approved
diff --git a/Documentation/releases/coreboot-4.13-relnotes.md b/Documentation/releases/coreboot-4.13-relnotes.md index 139fa20..c9447a3 100644 --- a/Documentation/releases/coreboot-4.13-relnotes.md +++ b/Documentation/releases/coreboot-4.13-relnotes.md @@ -88,4 +88,35 @@ It still needs changes in assembly, fixed integer to pointer conversions in C, wrappers for blobs, support for running Option ROMs, among other things.
+### Preparations to minimize enabling PCI bus mastering + +For security reasons, bus mastering should be enabled as late as possible. In +coreboot, it's usually not necessary and payloads should only enable it for +devices they use. Since not all payloads enable bus mastering properly yet, +some Kconfig options were added as an intermediate step to give some sort of +"backwards compatibility", which allow enabling or disabling bus mastering by +groups. + +Currently available groups are: + +* PCI bridges +* Any devices + +For now, "Any devices" is enabled by default to keep the traditional behaviour, +which also includes all other options. This is currently necessary, for instance, +for libpayload-based payloads as the drivers don't enable bus mastering for PCI +bridges. + +Exceptional cases, that may still need early bus master enabling in the future, +should get their own per-reason Kconfig option. Ideally before the next release. + ### Add significant changes here + +Deprecations +------------ + +### PCI bus master configuration options + +In order to minimize the usage of PCI bus mastering, the options we introduced in +this release will be dropped in a future release again. For more details, please +see [Preparations to minimize enabling PCI bus mastering](#preparations-to-minimize-enabling-pci-bus-mastering-in-coreboot).
-
Angel Pons (Code Review) -
Felix Singer (Code Review)
-
Michael Niewöhner (Code Review)
-
Nico Huber (Code Review)