Messages in this thread | | | Date | Thu, 14 May 2020 16:14:04 +0300 | From | Mika Westerberg <> | Subject | Re: [PATCH] mfd: Export LPC attributes for the system SPI chip |
| |
On Thu, May 14, 2020 at 01:53:23PM +0100, Richard Hughes wrote: > On Thu, 14 May 2020 at 13:15, Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > > +What: /sys/kernel/security/firmware/bioswe > > Should this still be "firmware_protections" or similar. Plain "firmware" > > sounds again too generic. Maybe its just me.. > > It's not always going to be protections provided by the firmware; it > might also be restrictions put on the firmware. My first choice was > /sys/kernel/security/firmware_security/ but having the double > 'security' just looked redundant.
OK.
> > > + LPC_SPT, /* Sunrise Point */ > > > + LPC_KBL, /* Kaby Lake */ > > > + LPC_TGL, /* Tiger Lake */ > > > > These all have the SPI-NOR controller as separate PCI device (as ICL and > > others). > > For Sunrise Point I see: > > 00:1f.0 ISA bridge [0601]: Intel Corporation CM236 Chipset LPC/eSPI > Controller [8086:a150] (rev 31) > 00:1f.0 ISA bridge [0601]: Intel Corporation Sunrise Point LPC > Controller/eSPI Controller [8086:9d4e] (rev 21) > > For Kaby Lake I see: > > 00:1f.0 ISA bridge [0601]: Intel Corporation HM175 Chipset LPC/eSPI > Controller [8086:a152] (rev 31)",
Yes, both of these have LPC device (1f.0) but the SPI-NOR controller is separate PCI device and most likely hidden.
> You're indeed correct about Tiger Lake, my apologies. > > > > + [LPC_SPT] = { > > > + .name = "Sunrise Point", > > > + .spi_type = INTEL_SPI_LPC, > > > + }, > > > > So all of these have LCP/eSPI controller but the SPI-NOR controller is > > not accessible through it - it is a separate PCI device. > > I have a Sunrise Point system here -- the lspci is here: > https://people.freedesktop.org/~hughsient/temp/lspci.txt > > Is the SPI-NOR controller perhaps hidden? If I read the BCR @ 0xdc > from the 00:1f.0 ISB bridge I get the expected BIOS_WE, BLE and > SMM_BWP results.
OK, I checked datasheet of KBL and indeed the LPC still has the BIOS Control (BC) register at 0xdc so that should work. Incidently the same register is part of the SPI-NOR controller register set.
> > Like you said, Evolution seems to mangle these. > > I'll use git for future patches, thanks. > > > > + pci_read_config_dword(dev, BCR, &bcr); > > > + info->writeable = !!(bcr & BCR_WPD); > > > + break; > > > + > > > + case INTEL_SPI_LPC: > > > > So instead of this, you can add the security attributes to the existing > > entries where we are sure there is SPI-NOR controller behind LPC. Here > > it is not the case and further.. > > Sooo I'd use INTEL_SPI_LPT? On my system RCBA isn't set, and so "if > (!res->start)" bails out with return -ENODEV;"
I think the INTEL_SPI_LPC is slightly misleading because the SPI is not accessible through LPC. Instead what if we read the BIOS control register first in lpc_ich_init_spi() and then bail out since .spi_type is not set?
Probably we can rename the function lpc_ich_init_spi() to lpc_ich_init_security_and_spi() or something like that.
| |