On Tue, Nov 3, 2020 at 1:07 PM Greg Kroah-Hartman <[email protected]> wrote: > > On Tue, Nov 03, 2020 at 12:18:01PM -0300, Daniel Gutson wrote: > > On Thu, Oct 29, 2020 at 4:14 PM Greg Kroah-Hartman > > <[email protected]> wrote: > > > > > > On Thu, Oct 29, 2020 at 12:39:08PM -0300, Daniel Gutson wrote: > > > > On Thu, Oct 29, 2020 at 2:40 AM Greg Kroah-Hartman > > > > <[email protected]> wrote: > > > > > > > > > > On Wed, Oct 28, 2020 at 06:43:59PM -0300, Daniel Gutson wrote: > > > > > > This patch separates the writing part of the intel-spi drivers > > > > > > so the 'dangerous' part can be set/unset independently. > > > > > > This way, the kernel can be configured to include the reading > > > > > > parts of the driver which can be used without > > > > > > the dangerous write operations that can turn the system > > > > > > unbootable. > > > > > > > > > > > > Signed-off-by: Daniel Gutson <[email protected]> > > > > > > --- > > > > > > drivers/mtd/spi-nor/controllers/Kconfig | 39 > > > > > > ++++++++++++--------- > > > > > > drivers/mtd/spi-nor/controllers/intel-spi.c | 12 +++++-- > > > > > > 2 files changed, 33 insertions(+), 18 deletions(-) > > > > > > > > > > > > diff --git a/drivers/mtd/spi-nor/controllers/Kconfig > > > > > > b/drivers/mtd/spi-nor/controllers/Kconfig > > > > > > index 5c0e0ec2e6d1..491c755fea49 100644 > > > > > > --- a/drivers/mtd/spi-nor/controllers/Kconfig > > > > > > +++ b/drivers/mtd/spi-nor/controllers/Kconfig > > > > > > @@ -31,34 +31,41 @@ config SPI_INTEL_SPI > > > > > > tristate > > > > > > > > > > > > config SPI_INTEL_SPI_PCI > > > > > > - tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)" > > > > > > + tristate "Intel PCH/PCU SPI flash PCI driver" > > > > > > depends on X86 && PCI > > > > > > select SPI_INTEL_SPI > > > > > > 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. > > > > > > - > > > > > > - Say N here unless you know what you are doing. Overwriting > > > > > > the > > > > > > - SPI flash may render the system unbootable. > > > > > > + This enables read only 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 read the SPI chip > > > > > > directly > > > > > > + from Linux. > > > > > > > > > > > > To compile this driver as a module, choose M here: the > > > > > > module > > > > > > will be called intel-spi-pci. > > > > > > > > > > > > config SPI_INTEL_SPI_PLATFORM > > > > > > - tristate "Intel PCH/PCU SPI flash platform driver (DANGEROUS)" > > > > > > + tristate "Intel PCH/PCU SPI flash platform driver" > > > > > > depends on X86 > > > > > > select SPI_INTEL_SPI > > > > > > help > > > > > > - This enables platform support for the Intel PCH/PCU SPI > > > > > > + This enables read only 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. > > > > > > + Intel hardware and is used to hold BIOS and other > > > > > > persistent settings. > > > > > > + Using this driver it is possible to read the SPI chip > > > > > > directly > > > > > > + from Linux. > > > > > > + > > > > > > + To compile this driver as a module, choose M here: the > > > > > > module > > > > > > + will be called intel-spi-pci. > > > > > > + > > > > > > +config SPI_INTEL_SPI_WRITE > > > > > > + bool "Intel PCH/PCU SPI flash drivers write operations > > > > > > (DANGEROUS)" > > > > > > + depends on SPI_INTEL_SPI_PCI || SPI_INTEL_SPI_PLATFORM > > > > > > + help > > > > > > + This enables full read/write support for the Intel PCH/PCU > > > > > > SPI > > > > > > + controller. > > > > > > + Using this option it may be 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. > > > > > > - > > > > > > - To compile this driver as a module, choose M here: the > > > > > > module > > > > > > - will be called intel-spi-platform. > > > > > > diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c > > > > > > b/drivers/mtd/spi-nor/controllers/intel-spi.c > > > > > > index b54a56a68100..8d8053395c3d 100644 > > > > > > --- a/drivers/mtd/spi-nor/controllers/intel-spi.c > > > > > > +++ b/drivers/mtd/spi-nor/controllers/intel-spi.c > > > > > > @@ -266,6 +266,7 @@ static int intel_spi_read_block(struct > > > > > > intel_spi *ispi, void *buf, size_t size) > > > > > > return 0; > > > > > > } > > > > > > > > > > > > +#ifdef CONFIG_SPI_INTEL_SPI_WRITE > > > > > > > > > > <snip> > > > > > > > > > > Please do not add #ifdef to .c files, that's not the proper kernel > > > > > coding style at all, and just makes maintaining this file much much > > > > > harder over time. > > > > > > > > > > Split things out into two different files if you really need to do > > > > > this. > > > > > > > > What about the static functions that I'll need to turn non-static and > > > > in a header file? > > > > I mean, the functions that the functions in the new file will have to > > > > call. > > > > Should I do that, turn static functions into non-static and declared > > > > in a header file? > > > > > > No idea, but again, no #ifdefs in .c files like this, that is not the > > > proper kernel coding style as it is not maintainable for the lifespan > > > that we have to maintain code. > > > > Is it acceptable to leave static functions unused and let the optimizer > > remove them as DCE? > > You will get build warnings if you do that, right?
I don't know why I didn't get any unused function warning, but OK. I'm using a decently recent gcc. > > You have thousands of examples of how to do this correct, it shouldn't > be that tough :) The job is not that tough, you are to accept changes :) > > thanks, > > greg k-h -- Daniel Gutson Engineering Director Eclypsium, Inc. Below The Surface: Get the latest threat research and insights on firmware and supply chain threats from the research team at Eclypsium. https://eclypsium.com/research/#threatreport
