Re: [PATCH v2 10/18] PCI/CMA: Reauthenticate devices on reset and resume

Jonathan Cameron
2024-07-18T15:01:38Z

On Wed, 10 Jul 2024 16:23:00 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Lukas Wunner wrote:
> > CMA-SPDM state is lost when a device undergoes a Conventional Reset.
> > (But not a Function Level Reset, PCIe r6.2 sec 6.6.2.) A D3cold to D0
> > transition implies a Conventional Reset (PCIe r6.2 sec 5.8).
> >
> > Thus, reauthenticate devices on resume from D3cold and on recovery from
> > a Secondary Bus Reset or DPC-induced Hot Reset.
> >
> > The requirement to reauthenticate devices on resume from system sleep
> > (and in the future reestablish IDE encryption) is the reason why SPDM
>
> TSM "connect" state also needs to be managed over reset, so stay tuned
> for some collaboration here.
>
> > needs to be in-kernel: During ->resume_noirq, which is the first phase
> > after system sleep, the PCI core walks down the hierarchy, puts each
> > device in D0, restores its config space and invokes the driver's
> > ->resume_noirq callback. The driver is afforded the right to access the
> > device already during this phase.
>
> I agree that CMA should be in kernel, it's not clear that authentication
> needs to be automatic, and certainly not in a way that a driver can not
> opt-out of.
>
> What if a use case cares about resume time latency? What if a driver
> knows that authentication is only needed later in the resume flow? Seems
> presumptious for the core to assume it knows best when authentication
> needs to happen.

Feels like a policy question - maybe a static key (as Kees suggested for
the other question). By all means default to on, but a latency sensitive
setup might opt out? Or specific driver opt out might be an option
if we are allowing a driver managed flow (and the policy allows drivers
to opt out - we definitely want a policy option that doesn't allow
drivers to be part of the decision and indeed does what we have here).

Hope someone writes a nice guide to any policy choices that come out of
this. Maybe the policy hooks don't belong in a first patch set though
as this one in particular is a performance optimization.

>
> At a minimum I think pci_cma_reauthenticate() should do something like:
>
> /* not previously authenticated skip authentication */
> if (!spdm_state->authenticated)
> return;
>
> ...so that spdm capable devices can opt-out of automatic reauthentication.

This seems reasonable as only possibility of change is that it can now
authenticate (maybe the reset was a firmware update...) and if we accepted
it before then no loss of security in not checking. Userspace can then poke
the reauthenticate and reload the driver if relevant (maybe more functionality
will be enabled.)

Note the whole always try to authenticate first was outcome of one of the LPC
BoFs (2 years ago?).

Jonathan

>