Re: [PATCH] firmware: export x86_64 platform flash bios region via sysfs

From: Mauro Lima <mauro.lima@eclypsium.com>
To: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Richard Hughes <hughsient@gmail.com>,
	Hans-Gert Dahmen <hans-gert.dahmen@immu.ne>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Philipp Deppenwiese <philipp.deppenwiese@immu.ne>,
	"platform-driver-x86@vger.kernel.org" 
	<platform-driver-x86@vger.kernel.org>
Subject: Re: [PATCH] firmware: export x86_64 platform flash bios region via sysfs
Date: Thu, 11 Nov 2021 12:31:16 -0300	[thread overview]
Message-ID: <CAArk9MMh575-JwrMrn=NF7hBk9t-mzjc7Uz1BwjWFZELsWtXXg@mail.gmail.com> (raw)
In-Reply-To: <YY0xZjjGjNq+kUXi@lahna>

On Thu, Nov 11, 2021 at 12:06 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Hi,
>
> On Thu, Nov 11, 2021 at 11:42:52AM -0300, Mauro Lima wrote:
> > > > > Having said that the hardware sequencer used in the recent CPUs should
> > > > > be much safer in that sense.
> > > >
> > > > FWIW, I'd be fine if we had RO access for HWSEQ flash access only. If
> > > > I understood correctly that's what Mauro proposed (with a patch) and
> > > > instead was told that it was being rewritten as a mtd driver
> > > > completion time unknown.
> > >
> > > I think Mauro proposed something different, basically exposing RO parts
> > > of the driver only.
> >
> > My patch was intended to move the read functionality of the spi chip
> > to be able to compile the driver with just that and then remove the
> > dangerous tag. So we can use that functionality to read the flash, I'm
> > missing what is different from the things being discussed here sorry.
>
> I'm hinting that we could make this "non-DANGEROUS" for hardware
> sequencer parts of the driver. Basically moving only the software
> sequencer bits as DANGEROUS or something like that. The hardware
> sequencer is much more safer because it does not allow to run random
> opcodes.

I'm aware about hw and sw sequencer diffs, my patch aimed to split
reading functionality because we thought that the issue was related
with write/erase ops plus the writable module param (this mixed with
the sw sequencer could be the bug?). I totally agree with the hw
sequencer path and I'm willing to help to make this happen.

> In case someone is unfamiliar with this, the Intel SPI hardware
> exposes two interfaces through the same controller. One that is called
> software sequencer and there is a register of "allowed" opcodes that
> software can use as it wishes. This register can be locked down but is
> not always. The second interface is the hardware sequencer that only
> exposes higher level commands like read, write and so on and internally
> then executes whatever opcode the controller got from the chip
> "supported opcodes table" (SFDP).  The recent Intel hardware, all
> big-cores, only provide hardware sequencer and the software one is not
> even available.
>
> Regardless of all this the driver needs to be converted from MTD to SPI
> (SPI MEM) before we can add any features. I'm planning to send v4 of
> that series next week.

Good luck with the patch.