Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <125a8c31e106dc68e6d3e3395cecc766db7fb897.camel@gmail.com>
 <20200513070847.GM2571@lahna.fi.intel.com> <CAD2FfiHsUjLC1K=HvF74LbRaKoc_zz6bOmGLQrQbW4CywWCP9A@mail.gmail.com>
 <20200513091100.GY2571@lahna.fi.intel.com> <CAD2FfiGNErUhz=7DH6Z37X573hSkJkzbOEXbb++X+Ey5WLc9=Q@mail.gmail.com>
 <20200513162513.GI2571@lahna.fi.intel.com> <c4602e6768377179ff3204ea0c2d5944e6d35844.camel@gmail.com>
 <20200514121536.GS2571@lahna.fi.intel.com>
In-Reply-To: <20200514121536.GS2571@lahna.fi.intel.com>
From:   Richard Hughes <hughsient@gmail.com>
Date:   Thu, 14 May 2020 13:53:23 +0100
Message-ID: <CAD2FfiEEzL0D5uRWgt=Hb6ngD2KY_NrZYJFAYtJi4CmS08zpfA@mail.gmail.com>
Subject: Re: [PATCH] mfd: Export LPC attributes for the system SPI chip
To:     Mika Westerberg <mika.westerberg@linux.intel.com>
Cc:     ptyser@xes-inc.com, Lee Jones <lee.jones@linaro.org>,
        tudor.ambarus@microchip.com,
        Kate Stewart <kstewart@linuxfoundation.org>,
        allison@lohutok.net, tglx@linutronix.de, jethro@fortanix.com,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.

> > +     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)",

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.

> 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;"

> Otherwise this looks good, nice work :)

I thank you for your patience so far, what I've got the hang of this I
promise to start being more useful.

Richard.