| Message ID | 20230206183143.75274-2-mauro.lima@eclypsium.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7db738b5fea4533fa217dfb05c506c15bd0964d9 |
| Headers | show |
| Series | spi: intel: Remove DANGEROUS tag from pci driver | expand |
Hi, One nit: In $subject please use PCI instead of pci. On Mon, Feb 06, 2023 at 03:31:43PM -0300, Mauro Lima wrote: > Modern CPUs exposes this controller as PCI device that only uses > hardware sequencing capabilities which is safer than software > sequencing. > Leave the platform driver as *DANGEROUS* and update help text since > most of these controllers are using software sequencing. > > Signed-off-by: Mauro Lima <mauro.lima@eclypsium.com> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Modern CPUs exposes this controller as PCI device that only uses > hardware sequencing capabilities which is safer than software > sequencing. > Leave the platform driver as *DANGEROUS* and update help text since > most of these controllers are using software sequencing. Out of curiosity, what is hardware sequencing? Maybe this should be explained a bit more in the Kconfig help text. Looks like the dangerous was there because you can update the bios and that could eventually lead to a bricked mainboard. So hardware sequencing helps there? how? -michael
Hi, On Tue, Feb 07, 2023 at 02:52:54PM +0100, Michael Walle wrote: > > Modern CPUs exposes this controller as PCI device that only uses > > hardware sequencing capabilities which is safer than software > > sequencing. > > Leave the platform driver as *DANGEROUS* and update help text since > > most of these controllers are using software sequencing. > > Out of curiosity, what is hardware sequencing? Maybe this should > be explained a bit more in the Kconfig help text. Looks like the > dangerous was there because you can update the bios and that > could eventually lead to a bricked mainboard. So hardware > sequencing helps there? how? Hardware sequencing means the controller exposes just a bunch of "high level" operations to the software. Such as read, write, erase and so on but does not allow running the actual "low level" SPI-NOR opcodes. Software sequencing on the other hand allows running pretty much any opcode and this is what caused problems for certain Lenovo laptops few years back that then resulted adding DANGEROUS to the Kconfig. Typically the flash is locked by the BIOS so ordinary users cannot really overwrite it, even by accident.
Hi Mika, Am 2023-02-07 15:03, schrieb Mika Westerberg: > On Tue, Feb 07, 2023 at 02:52:54PM +0100, Michael Walle wrote: >> > Modern CPUs exposes this controller as PCI device that only uses >> > hardware sequencing capabilities which is safer than software >> > sequencing. >> > Leave the platform driver as *DANGEROUS* and update help text since >> > most of these controllers are using software sequencing. >> >> Out of curiosity, what is hardware sequencing? Maybe this should >> be explained a bit more in the Kconfig help text. Looks like the >> dangerous was there because you can update the bios and that >> could eventually lead to a bricked mainboard. So hardware >> sequencing helps there? how? > > Hardware sequencing means the controller exposes just a bunch of "high > level" operations to the software. Ok, I figured it would have been something to do with the SPI driver just supporting these high level ops. But even with that background it was hard to connect that to the "hardware sequencing". The help text should be somewhat understandable to the user/distro people/whoever, right? So I'd suggest to explain that a bit more in detail, or don't use the term hardware sequencing at all. I'm not sure. > Such as read, write, erase and so on > but does not allow running the actual "low level" SPI-NOR opcodes. > Software sequencing on the other hand allows running pretty much any > opcode and this is what caused problems for certain Lenovo laptops few > years back that then resulted adding DANGEROUS to the Kconfig. That information should go into the commit message. > Typically the flash is locked by the BIOS so ordinary users cannot > really overwrite it, even by accident. I see, thanks for the explanation! -michael
Hi, On Tue, Feb 07, 2023 at 03:11:26PM +0100, Michael Walle wrote: > Hi Mika, > > Am 2023-02-07 15:03, schrieb Mika Westerberg: > > On Tue, Feb 07, 2023 at 02:52:54PM +0100, Michael Walle wrote: > > > > Modern CPUs exposes this controller as PCI device that only uses > > > > hardware sequencing capabilities which is safer than software > > > > sequencing. > > > > Leave the platform driver as *DANGEROUS* and update help text since > > > > most of these controllers are using software sequencing. > > > > > > Out of curiosity, what is hardware sequencing? Maybe this should > > > be explained a bit more in the Kconfig help text. Looks like the > > > dangerous was there because you can update the bios and that > > > could eventually lead to a bricked mainboard. So hardware > > > sequencing helps there? how? > > > > Hardware sequencing means the controller exposes just a bunch of "high > > level" operations to the software. > > Ok, I figured it would have been something to do with the SPI driver > just supporting these high level ops. But even with that background > it was hard to connect that to the "hardware sequencing". The help > text should be somewhat understandable to the user/distro people/whoever, > right? So I'd suggest to explain that a bit more in detail, or don't > use the term hardware sequencing at all. I'm not sure. I agree it should be made more understandable for the distro folks. At least add some explanation why it is OK to select this. Mauro, can you do that in the next version? > > Such as read, write, erase and so on > > but does not allow running the actual "low level" SPI-NOR opcodes. > > Software sequencing on the other hand allows running pretty much any > > opcode and this is what caused problems for certain Lenovo laptops few > > years back that then resulted adding DANGEROUS to the Kconfig. > > That information should go into the commit message. +1
Hi all, On Tue, Feb 7, 2023 at 2:25 PM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > Hi, > > On Tue, Feb 07, 2023 at 03:11:26PM +0100, Michael Walle wrote: > > Hi Mika, > > > > Am 2023-02-07 15:03, schrieb Mika Westerberg: > > > On Tue, Feb 07, 2023 at 02:52:54PM +0100, Michael Walle wrote: > > > > > Modern CPUs exposes this controller as PCI device that only uses > > > > > hardware sequencing capabilities which is safer than software > > > > > sequencing. > > > > > Leave the platform driver as *DANGEROUS* and update help text since > > > > > most of these controllers are using software sequencing. > > > > > > > > Out of curiosity, what is hardware sequencing? Maybe this should > > > > be explained a bit more in the Kconfig help text. Looks like the > > > > dangerous was there because you can update the bios and that > > > > could eventually lead to a bricked mainboard. So hardware > > > > sequencing helps there? how? > > > > > > Hardware sequencing means the controller exposes just a bunch of "high > > > level" operations to the software. > > > > Ok, I figured it would have been something to do with the SPI driver > > just supporting these high level ops. But even with that background > > it was hard to connect that to the "hardware sequencing". The help > > text should be somewhat understandable to the user/distro people/whoever, > > right? So I'd suggest to explain that a bit more in detail, or don't > > use the term hardware sequencing at all. I'm not sure. > > I agree it should be made more understandable for the distro folks. At > least add some explanation why it is OK to select this. I agree with this. > Mauro, can you do that in the next version? Sure thing. > > > Such as read, write, erase and so on > > > but does not allow running the actual "low level" SPI-NOR opcodes. > > > Software sequencing on the other hand allows running pretty much any > > > opcode and this is what caused problems for certain Lenovo laptops few > > > years back that then resulted adding DANGEROUS to the Kconfig. > > > > That information should go into the commit message. > > +1 Sorry about this, still learning :) Thanks all for your comments and time.
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 87fc2bd16b72..3a362c450cb6 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -447,7 +447,7 @@ config SPI_INTEL tristate config SPI_INTEL_PCI - tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)" + tristate "Intel PCH/PCU SPI flash PCI driver" depends on PCI depends on X86 || COMPILE_TEST depends on SPI_MEM @@ -455,8 +455,9 @@ config SPI_INTEL_PCI help This enables PCI support for the Intel PCH/PCU SPI controller in master mode. This controller is present in modern Intel hardware - and is used to hold BIOS and other persistent settings. Using - this driver it is possible to upgrade BIOS directly from Linux. + and is used to hold BIOS and other persistent settings. This + driver only supports hardware sequencing mode. Using this + driver it is possible to upgrade BIOS directly from Linux. Say N here unless you know what you are doing. Overwriting the SPI flash may render the system unbootable. @@ -471,10 +472,10 @@ config SPI_INTEL_PLATFORM select SPI_INTEL help This enables platform support for the Intel PCH/PCU SPI - controller in master mode. This controller is present in modern - Intel hardware and is used to hold BIOS and other persistent - settings. Using this driver it is possible to upgrade BIOS - directly from Linux. + controller in master mode that is used to hold BIOS and other + persistent settings. Most of these controllers are using + software sequencing mode. Using this driver it is possible to + upgrade BIOS directly from Linux. Say N here unless you know what you are doing. Overwriting the SPI flash may render the system unbootable.
Modern CPUs exposes this controller as PCI device that only uses hardware sequencing capabilities which is safer than software sequencing. Leave the platform driver as *DANGEROUS* and update help text since most of these controllers are using software sequencing. Signed-off-by: Mauro Lima <mauro.lima@eclypsium.com> --- drivers/spi/Kconfig | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)