Deprecating spurious PCI bus master enabling (was Re: Planning the next coreboot release)
Hi, On 04.11.20 23:21, Angel Pons wrote:
3. Please take a look at the preliminary release notes in Documentation/releases/coreboot-4.13-relnotes.md and add whatever happened since 4.12 that is worth mentioning. If unsure, simply push a change to Gerrit and have your fellow developers discuss it.
I have a candidate for a deprecation warning: We're currently working to make all not (clearly) necessary PCI bus master enabling optional. The added Kconfig option still defaults to `y` [1], for compatibility reasons. This has security implications and hence I propose to proceed as follows: After the follow-up (spring) release, * remove the option and the code using it, or at least * reduce its usage to a minimum and change the default to `n`. This should give people enough time to test (with the option disabled) and potentially fix their payloads (and OS?). If there are no concerns, I'll add that to the release notes. There are cases where enabling PCI bus mastering already in coreboot might be necessary. For instance, Intel documents for their integrated graphics to enable it before the initialization sequence (without tel- ling us why and tests show that it ignores the bit anyway *shrug*). And also cases where it's clearly necessary, e.g. before hiding a PCI devices configuration space to put it into ACPI mode (if the OS is not aware of the device' PCI nature, it can't know that bus mastering needs to be enabled). If anybody knows or discovers more cases where it needs to be enabled in advance by coreboot, please mention it here. Nico [1] https://review.coreboot.org/c/coreboot/+/42459
On 10.11.20 16:06, Nico Huber wrote:
If anybody knows or discovers more cases where it needs to be enabled in advance by coreboot, please mention it here.
We just discussed on IRC cases where unfixable OS drivers might need it. For such cases, it would probably be best to add individual Kconfig switches for each case. This would make it easier to get rid of the big switch. Nico
nice idea! On Tue, Nov 10, 2020 at 7:13 AM Nico Huber <nico.h@gmx.de> wrote:
On 10.11.20 16:06, Nico Huber wrote:
If anybody knows or discovers more cases where it needs to be enabled in advance by coreboot, please mention it here.
We just discussed on IRC cases where unfixable OS drivers might need it. For such cases, it would probably be best to add individual Kconfig switches for each case. This would make it easier to get rid of the big switch.
Nico _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
We could introduce a Kconfig switch per driver and let the driver handle the bit. Everything else could be removed. This would make it easier to track the usages. It would be nice if we could agree on a naming scheme so that all switches are named similar which would make it easier to track the usage. But there are cases where there is no driver in coreboot for a given PCI device which needs this bit. For now, we (Siemens) handle this cases on mainboard level. So either we need drivers for these devices (just simply setting the master bit) or we can agree on some kind of exceptions. I am open to everything. Werner -----Ursprüngliche Nachricht----- Von: ron minnich <rminnich@gmail.com> Gesendet: Dienstag, 10. November 2020 16:15 An: Nico Huber <nico.h@gmx.de> Cc: coreboot <coreboot@coreboot.org> Betreff: [coreboot] Re: Deprecating spurious PCI bus master enabling (was Re: Planning the next coreboot release) nice idea! On Tue, Nov 10, 2020 at 7:13 AM Nico Huber <nico.h@gmx.de> wrote:
On 10.11.20 16:06, Nico Huber wrote:
If anybody knows or discovers more cases where it needs to be enabled in advance by coreboot, please mention it here.
We just discussed on IRC cases where unfixable OS drivers might need it. For such cases, it would probably be best to add individual Kconfig switches for each case. This would make it easier to get rid of the big switch.
Nico _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
_______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
Dear coreboot folks, Am 10.11.20 um 16:23 schrieb werner.zeh@siemens.com:
We could introduce a Kconfig switch per driver and let the driver handle the bit. Everything else could be removed. This would make it easier to track the usages. It would be nice if we could agree on a naming scheme so that all switches are named similar which would make it easier to track the usage.
But there are cases where there is no driver in coreboot for a given PCI device which needs this bit. For now, we (Siemens) handle this cases on mainboard level. So either we need drivers for these devices (just simply setting the master bit) or we can agree on some kind of exceptions. I am open to everything. Two more suggestions:
1. Log a warning for all those cases, that this might have security implications a.nd that it should be made sure, that the payload and OS do it themselves. Warning: Bus master enabled for …. This impacts security, so ideally make sure your payload and OS is able to set it themselves. 2. Make it run-time configurable for easier testing. Kind regards, Paul
Hi Werner, Ron, Nico, list, While it would be great to not have to implement Bus Master workarounds in coreboot, I guess it's sometimes unavoidable because of external constraints. I would much prefer to have the actual problem fixed instead (code assuming Bus Master is enabled by default), but if that's not possible I can live with some mechanism to enable Bus Master in coreboot, *as long as all usages are justified*. IMHO, enabling Bus Master for no reason is bad practice, but keeping code to enable Bus Master because no one knows what removing it could break is even worse. On Tue, Nov 10, 2020 at 3:24 PM werner.zeh@siemens.com <werner.zeh@siemens.com> wrote:
We could introduce a Kconfig switch per driver
Not per-driver, please! I'd rather have per-reason switches (reason-options): Kconfig options whose help-text gives the reason why enabling Bus Master in coreboot for certain devices is needed. Even if these Kconfig options end up being scattered across the tree, it's easier to see the relationship between multiple instances of Bus Master being set. Plus, if the actual bug is eventually fixed, it's very easy to drop the corresponding reason-option without breaking any other reason-options.
and let the driver handle the bit. Everything else could be removed. This would make it easier to track the usages. It would be nice if we could agree on a naming scheme so that all switches are named similar which would make it easier to track the usage.
But there are cases where there is no driver in coreboot for a given PCI device which needs this bit. For now, we (Siemens) handle this cases on mainboard level. So either we need drivers for these devices (just simply setting the master bit) or we can agree on some kind of exceptions. I am open to everything.
This won't be a problem if using per-reason Kconfig options. I would still use these reason-options though, even when the option always gets selected (e.g. selected by the mainboard's Kconfig): this is to justify each instance of Bus Master being set with the corresponding reason-options.
Werner
-----Ursprüngliche Nachricht----- Von: ron minnich <rminnich@gmail.com> Gesendet: Dienstag, 10. November 2020 16:15 An: Nico Huber <nico.h@gmx.de> Cc: coreboot <coreboot@coreboot.org> Betreff: [coreboot] Re: Deprecating spurious PCI bus master enabling (was Re: Planning the next coreboot release)
nice idea!
On Tue, Nov 10, 2020 at 7:13 AM Nico Huber <nico.h@gmx.de> wrote:
On 10.11.20 16:06, Nico Huber wrote:
If anybody knows or discovers more cases where it needs to be enabled in advance by coreboot, please mention it here.
We just discussed on IRC cases where unfixable OS drivers might need it. For such cases, it would probably be best to add individual Kconfig switches for each case. This would make it easier to get rid of the big switch.
Nico _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
_______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
Best regards, Angel
participants (5)
-
Angel Pons -
Nico Huber -
Paul Menzel -
ron minnich -
werner.zeh＠siemens.com