mbox series

[00/12] PCI device authentication

Message ID cover.1695921656.git.lukas@wunner.de
Headers show
Series PCI device authentication | expand

Message

Lukas Wunner Sept. 28, 2023, 5:32 p.m. UTC
Authenticate PCI devices with CMA-SPDM (PCIe r6.1 sec 6.31) and
expose the result in sysfs.  This enables user-defined policies
such as forbidding driver binding to devices which failed
authentication.

CMA-SPDM forms the basis for PCI encryption (PCIe r6.1 sec 6.33),
which will be submitted later.

The meat of the series is in patches [07/12] and [08/12], which contain
the SPDM library and the CMA glue code (the PCI-adaption of SPDM).

The reason why SPDM is done in-kernel is provided in patch [10/12]:
Briefly, when devices are reauthenticated on resume from system sleep,
user space is not yet available.  Same when reauthenticating after
recovery from reset.

One use case for CMA-SPDM and PCI encryption is confidential access
to passed-through devices:  Neither the host nor other guests are
able to eavesdrop on device accesses, in particular if guest memory
is encrypted as well.

Further use cases for the SPDM library are appearing on the horizon:
Alistair Francis and Wilfred Mallawa from WDC are interested in using
it for SCSI/SATA.  David Box from Intel has implemented measurement
retrieval over SPDM.

The root of trust is initially an in-kernel key ring of certificates.
We can discuss linking the system key ring into it, thereby allowing
EFI to pass trusted certificates to the kernel for CMA.  Alternatively,
a bundle of trusted certificates could be loaded from the initrd.
I envision that we'll add TPMs or remote attestation services such as
https://keylime.dev/ to create an ecosystem of various trust sources.

If you wish to play with PCI device authentication but lack capable
hardware, Wilfred has written a guide how to test with qemu:
https://github.com/twilfredo/spdm-emulation-guide-b

Jonathan Cameron (2):
  spdm: Introduce library to authenticate devices
  PCI/CMA: Authenticate devices on enumeration

Lukas Wunner (10):
  X.509: Make certificate parser public
  X.509: Parse Subject Alternative Name in certificates
  X.509: Move certificate length retrieval into new helper
  certs: Create blacklist keyring earlier
  crypto: akcipher - Support more than one signature encoding
  crypto: ecdsa - Support P1363 signature encoding
  PCI/CMA: Validate Subject Alternative Name in certificates
  PCI/CMA: Reauthenticate devices on reset and resume
  PCI/CMA: Expose in sysfs whether devices are authenticated
  PCI/CMA: Grant guests exclusive control of authentication

 Documentation/ABI/testing/sysfs-bus-pci   |   27 +
 MAINTAINERS                               |   10 +
 certs/blacklist.c                         |    4 +-
 crypto/akcipher.c                         |    2 +-
 crypto/asymmetric_keys/public_key.c       |   12 +-
 crypto/asymmetric_keys/x509_cert_parser.c |   15 +
 crypto/asymmetric_keys/x509_loader.c      |   38 +-
 crypto/asymmetric_keys/x509_parser.h      |   37 +-
 crypto/ecdsa.c                            |   16 +-
 crypto/internal.h                         |    1 +
 crypto/rsa-pkcs1pad.c                     |   11 +-
 crypto/sig.c                              |    6 +-
 crypto/testmgr.c                          |    8 +-
 crypto/testmgr.h                          |   16 +
 drivers/pci/Kconfig                       |   16 +
 drivers/pci/Makefile                      |    5 +
 drivers/pci/cma-sysfs.c                   |   73 +
 drivers/pci/cma-x509.c                    |  119 ++
 drivers/pci/cma.asn1                      |   36 +
 drivers/pci/cma.c                         |  151 +++
 drivers/pci/doe.c                         |    5 +-
 drivers/pci/pci-driver.c                  |    1 +
 drivers/pci/pci-sysfs.c                   |    3 +
 drivers/pci/pci.c                         |   12 +-
 drivers/pci/pci.h                         |   17 +
 drivers/pci/pcie/err.c                    |    3 +
 drivers/pci/probe.c                       |    1 +
 drivers/pci/remove.c                      |    1 +
 drivers/vfio/pci/vfio_pci_core.c          |    9 +-
 include/crypto/akcipher.h                 |   10 +-
 include/crypto/sig.h                      |    6 +-
 include/keys/asymmetric-type.h            |    2 +
 include/keys/x509-parser.h                |   46 +
 include/linux/oid_registry.h              |    3 +
 include/linux/pci-doe.h                   |    4 +
 include/linux/pci.h                       |   15 +
 include/linux/spdm.h                      |   41 +
 lib/Kconfig                               |   15 +
 lib/Makefile                              |    2 +
 lib/spdm_requester.c                      | 1510 +++++++++++++++++++++
 40 files changed, 2232 insertions(+), 77 deletions(-)
 create mode 100644 drivers/pci/cma-sysfs.c
 create mode 100644 drivers/pci/cma-x509.c
 create mode 100644 drivers/pci/cma.asn1
 create mode 100644 drivers/pci/cma.c
 create mode 100644 include/keys/x509-parser.h
 create mode 100644 include/linux/spdm.h
 create mode 100644 lib/spdm_requester.c

Comments

Dan Williams Oct. 6, 2023, 4:06 p.m. UTC | #1
This looks great Lukas, some forward looking review comments below.

Lukas Wunner wrote:
> Authenticate PCI devices with CMA-SPDM (PCIe r6.1 sec 6.31) and
> expose the result in sysfs.  This enables user-defined policies
> such as forbidding driver binding to devices which failed
> authentication.
> 
> CMA-SPDM forms the basis for PCI encryption (PCIe r6.1 sec 6.33),
> which will be submitted later.
> 
> The meat of the series is in patches [07/12] and [08/12], which contain
> the SPDM library and the CMA glue code (the PCI-adaption of SPDM).
> 
> The reason why SPDM is done in-kernel is provided in patch [10/12]:
> Briefly, when devices are reauthenticated on resume from system sleep,
> user space is not yet available.  Same when reauthenticating after
> recovery from reset.
> 
> One use case for CMA-SPDM and PCI encryption is confidential access
> to passed-through devices:  Neither the host nor other guests are
> able to eavesdrop on device accesses, in particular if guest memory
> is encrypted as well.

Note, only for traffic over the SPDM session. In order for private MMIO and
T=1 traffic to private memory, coordination with the platform TSM is
mandated by all the known TSM (CPU/Platform security modules). This has
implications for policy decisions later in this series.

> Further use cases for the SPDM library are appearing on the horizon:
> Alistair Francis and Wilfred Mallawa from WDC are interested in using
> it for SCSI/SATA.  David Box from Intel has implemented measurement
> retrieval over SPDM.
> 
> The root of trust is initially an in-kernel key ring of certificates.
> We can discuss linking the system key ring into it, thereby allowing
> EFI to pass trusted certificates to the kernel for CMA.  Alternatively,
> a bundle of trusted certificates could be loaded from the initrd.
> I envision that we'll add TPMs or remote attestation services such as
> https://keylime.dev/ to create an ecosystem of various trust sources.

Linux also has an interest in accommodating opt-in to using platform
managed keys, so the design requires that key management and session
ownership is a system owner policy choice.

> If you wish to play with PCI device authentication but lack capable
> hardware, Wilfred has written a guide how to test with qemu:
> https://github.com/twilfredo/spdm-emulation-guide-b
>
Lukas Wunner Oct. 7, 2023, 10:04 a.m. UTC | #2
On Fri, Oct 06, 2023 at 09:06:13AM -0700, Dan Williams wrote:
> Lukas Wunner wrote:
> > The root of trust is initially an in-kernel key ring of certificates.
> > We can discuss linking the system key ring into it, thereby allowing
> > EFI to pass trusted certificates to the kernel for CMA.  Alternatively,
> > a bundle of trusted certificates could be loaded from the initrd.
> > I envision that we'll add TPMs or remote attestation services such as
> > https://keylime.dev/ to create an ecosystem of various trust sources.
> 
> Linux also has an interest in accommodating opt-in to using platform
> managed keys, so the design requires that key management and session
> ownership is a system owner policy choice.

You're pointing out a gap in the specification:

There's an existing mechanism to negotiate which PCI features are
handled natively by the OS and which by platform firmware and that's
the _OSC Control Field (PCI Firmware Spec r3.3 table 4-5 and 4-6).

There are currently 10 features whose ownership is negotiated with _OSC,
examples are Hotplug control and DPC configuration control.

I propose adding an 11th bit to negotiate ownership of the CMA-SPDM
session.

Once that's added to the PCI Firmware Spec, amending the implementation
to honor it is trivial:  Just check for platform ownership at the top
of pci_cma_init() and return.

Thanks,

Lukas
Jonathan Cameron Oct. 9, 2023, 11:33 a.m. UTC | #3
On Sat, 7 Oct 2023 12:04:33 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> On Fri, Oct 06, 2023 at 09:06:13AM -0700, Dan Williams wrote:
> > Lukas Wunner wrote:  
> > > The root of trust is initially an in-kernel key ring of certificates.
> > > We can discuss linking the system key ring into it, thereby allowing
> > > EFI to pass trusted certificates to the kernel for CMA.  Alternatively,
> > > a bundle of trusted certificates could be loaded from the initrd.
> > > I envision that we'll add TPMs or remote attestation services such as
> > > https://keylime.dev/ to create an ecosystem of various trust sources.  
> > 
> > Linux also has an interest in accommodating opt-in to using platform
> > managed keys, so the design requires that key management and session
> > ownership is a system owner policy choice.  
> 
> You're pointing out a gap in the specification:
> 
> There's an existing mechanism to negotiate which PCI features are
> handled natively by the OS and which by platform firmware and that's
> the _OSC Control Field (PCI Firmware Spec r3.3 table 4-5 and 4-6).
> 
> There are currently 10 features whose ownership is negotiated with _OSC,
> examples are Hotplug control and DPC configuration control.
> 
> I propose adding an 11th bit to negotiate ownership of the CMA-SPDM
> session.
> 
> Once that's added to the PCI Firmware Spec, amending the implementation
> to honor it is trivial:  Just check for platform ownership at the top
> of pci_cma_init() and return.

This might want to be a control over the specific DOE instance instead
of a general purpose CMA control (or maybe we want both).

There is no safe way to access a DOE to find out if it supports CMA
that doesn't potentially break another entity using the mailbox.
Given the DOE instances might be for something entirely different we
can't just decide not to use them at all based on a global control.

Any such control becomes messy when hotplug is taken into account.
I suppose we could do a _DSM based on BDF / path to device (to remain
stable across reenumeration) and config space offset to allow the OS
to say 'Hi other entity / firmware are you using this DOE instance?"
Kind of an OSC with parameters.  Also includes the other way around that
the question tells the firmware that if it says "no you can't" the OS
will leave it alone until a reboot or similar - that potentially avoids
the problem that we access DOE instances already without taking care
about this (I dropped ball on this having raised it way back near start
of us adding DOE support.)

If we do want to do any of these, which spec is appropriate?  Link it to PCI
and propose a PCI firmware spec update? (not sure they have a code
first process available) or make it somewhat generic and propose an
ACPI Code first change?

Jonathan

> 
> Thanks,
> 
> Lukas
> 
>
Lukas Wunner Oct. 9, 2023, 1:49 p.m. UTC | #4
On Mon, Oct 09, 2023 at 12:33:35PM +0100, Jonathan Cameron wrote:
> On Sat, 7 Oct 2023 12:04:33 +0200 Lukas Wunner <lukas@wunner.de> wrote:
> > On Fri, Oct 06, 2023 at 09:06:13AM -0700, Dan Williams wrote:
> > > Linux also has an interest in accommodating opt-in to using platform
> > > managed keys, so the design requires that key management and session
> > > ownership is a system owner policy choice.  
> > 
> > You're pointing out a gap in the specification:
> > 
> > There's an existing mechanism to negotiate which PCI features are
> > handled natively by the OS and which by platform firmware and that's
> > the _OSC Control Field (PCI Firmware Spec r3.3 table 4-5 and 4-6).
> > 
> > There are currently 10 features whose ownership is negotiated with _OSC,
> > examples are Hotplug control and DPC configuration control.
> > 
> > I propose adding an 11th bit to negotiate ownership of the CMA-SPDM
> > session.
> > 
> > Once that's added to the PCI Firmware Spec, amending the implementation
> > to honor it is trivial:  Just check for platform ownership at the top
> > of pci_cma_init() and return.
> 
> This might want to be a control over the specific DOE instance instead
> of a general purpose CMA control (or maybe we want both).
> 
> There is no safe way to access a DOE to find out if it supports CMA
> that doesn't potentially break another entity using the mailbox.
> Given the DOE instances might be for something entirely different we
> can't just decide not to use them at all based on a global control.

Per PCIe r6.1 sec 6.31.3, the DOE instance used for CMA-SPDM must support
"no other data object protocol(s)" besides DOE discovery, CMA-SPDM and
Secured CMA-SPDM.

So if the platform doesn't grant the OS control over that DOE instance,
unrelated DOE instances and protocols (such as CDAT retrieval) are not
affected.

E.g. PCI Firmware Spec r3.3 table 4-5 could be amended with something
along the lines of:

  Control Field Bit Offset: 11

  Interpretation: PCI Express Component Measurement and Authentication control

  The operating system sets this bit to 1 to request control over the
  DOE instance supporting the CMA-SPDM feature.

You're right that to discover the DOE instance for CMA-SPDM in the
first place, it needs to be accessed, which might interfere with the
firmware using it.  Perhaps this can be solved with the DOE Busy bit.


> Any such control becomes messy when hotplug is taken into account.
> I suppose we could do a _DSM based on BDF / path to device (to remain
> stable across reenumeration) and config space offset to allow the OS
> to say 'Hi other entity / firmware are you using this DOE instance?"
> Kind of an OSC with parameters.  Also includes the other way around that
> the question tells the firmware that if it says "no you can't" the OS
> will leave it alone until a reboot or similar - that potentially avoids
> the problem that we access DOE instances already without taking care
> about this

PCI Firmware Spec r3.3 table 4-7 lists a number of _DSM Definitions for
PCI.  Indeed that could be another solution.  E.g. a newly defined _DSM
might return the offset in config space of DOE instance(s) which the OS
is not permitted to use.


> (I dropped ball on this having raised it way back near start
> of us adding DOE support.)

Not your fault.  I think the industry got a bit ahead of itself in
its "confidential computing" frenzy and forgot to specify these very
basic things.


> If we do want to do any of these, which spec is appropriate?  Link it to PCI
> and propose a PCI firmware spec update? (not sure they have a code
> first process available) or make it somewhat generic and propose an
> ACPI Code first change?

PCI Firmware Spec would seem to be appropriate.  However this can't
be solved by the kernel community.  We need to talk to our confidential
computing architects and our representatives at the PCISIG to get the
spec amended.

Thanks,

Lukas
Alexey Kardashevskiy Oct. 10, 2023, 4:07 a.m. UTC | #5
On 10/10/23 00:49, Lukas Wunner wrote:
> On Mon, Oct 09, 2023 at 12:33:35PM +0100, Jonathan Cameron wrote:
>> On Sat, 7 Oct 2023 12:04:33 +0200 Lukas Wunner <lukas@wunner.de> wrote:
>>> On Fri, Oct 06, 2023 at 09:06:13AM -0700, Dan Williams wrote:
>>>> Linux also has an interest in accommodating opt-in to using platform
>>>> managed keys, so the design requires that key management and session
>>>> ownership is a system owner policy choice.
>>>
>>> You're pointing out a gap in the specification:
>>>
>>> There's an existing mechanism to negotiate which PCI features are
>>> handled natively by the OS and which by platform firmware and that's
>>> the _OSC Control Field (PCI Firmware Spec r3.3 table 4-5 and 4-6).
>>>
>>> There are currently 10 features whose ownership is negotiated with _OSC,
>>> examples are Hotplug control and DPC configuration control.
>>>
>>> I propose adding an 11th bit to negotiate ownership of the CMA-SPDM
>>> session.
>>>
>>> Once that's added to the PCI Firmware Spec, amending the implementation
>>> to honor it is trivial:  Just check for platform ownership at the top
>>> of pci_cma_init() and return.
>>
>> This might want to be a control over the specific DOE instance instead
>> of a general purpose CMA control (or maybe we want both).
>>
>> There is no safe way to access a DOE to find out if it supports CMA
>> that doesn't potentially break another entity using the mailbox.
>> Given the DOE instances might be for something entirely different we
>> can't just decide not to use them at all based on a global control.
> 
> Per PCIe r6.1 sec 6.31.3, the DOE instance used for CMA-SPDM must support
> "no other data object protocol(s)" besides DOE discovery, CMA-SPDM and
> Secured CMA-SPDM.
> 
> So if the platform doesn't grant the OS control over that DOE instance,
> unrelated DOE instances and protocols (such as CDAT retrieval) are not
> affected.
> 
> E.g. PCI Firmware Spec r3.3 table 4-5 could be amended with something
> along the lines of:
> 
>    Control Field Bit Offset: 11
> 
>    Interpretation: PCI Express Component Measurement and Authentication control
> 
>    The operating system sets this bit to 1 to request control over the
>    DOE instance supporting the CMA-SPDM feature.
> 
> You're right that to discover the DOE instance for CMA-SPDM in the
> first place, it needs to be accessed, which might interfere with the
> firmware using it.  Perhaps this can be solved with the DOE Busy bit.
> 
> 
>> Any such control becomes messy when hotplug is taken into account.
>> I suppose we could do a _DSM based on BDF / path to device (to remain
>> stable across reenumeration) and config space offset to allow the OS
>> to say 'Hi other entity / firmware are you using this DOE instance?"
>> Kind of an OSC with parameters.  Also includes the other way around that
>> the question tells the firmware that if it says "no you can't" the OS
>> will leave it alone until a reboot or similar - that potentially avoids
>> the problem that we access DOE instances already without taking care
>> about this
> 
> PCI Firmware Spec r3.3 table 4-7 lists a number of _DSM Definitions for
> PCI.  Indeed that could be another solution.  E.g. a newly defined _DSM
> might return the offset in config space of DOE instance(s) which the OS
> is not permitted to use.
> 
> 
>> (I dropped ball on this having raised it way back near start
>> of us adding DOE support.)
> 
> Not your fault.  I think the industry got a bit ahead of itself in
> its "confidential computing" frenzy and forgot to specify these very
> basic things.
> 
> 
>> If we do want to do any of these, which spec is appropriate?  Link it to PCI
>> and propose a PCI firmware spec update? (not sure they have a code
>> first process available) or make it somewhat generic and propose an
>> ACPI Code first change?
> 
> PCI Firmware Spec would seem to be appropriate.  However this can't
> be solved by the kernel community.

How so? It is up to the user to decide whether it is SPDM/CMA in the 
kernel   or   the firmware + coco, both are quite possible (it is IDE 
which is not possible without the firmware on AMD but we are not there yet).

But the way SPDM is done now is that if the user (as myself) wants to 
let the firmware run SPDM - the only choice is disabling CONFIG_CMA 
completely as CMA is not a (un)loadable module or built-in (with some 
"blacklist" parameters), and does not provide a sysfs knob to control 
its tentacles. Kinda harsh.

Note, this PSP firmware is not BIOS (which runs on the same core and has 
same access to PCI as the host OS), it is a separate platform processor 
which only programs IDE keys to the PCI RC (via some some internal bus 
mechanism) but does not do anything on the bus itself and relies on the 
host OS proxying DOE, and there is no APCI between the core and the psp.


>  We need to talk to our confidential
> computing architects and our representatives at the PCISIG to get the
> spec amended.
> 
> Thanks,
> 
> Lukas
Lukas Wunner Oct. 10, 2023, 8:19 a.m. UTC | #6
On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:
> On 10/10/23 00:49, Lukas Wunner wrote:
> > PCI Firmware Spec would seem to be appropriate.  However this can't
> > be solved by the kernel community.
> 
> How so? It is up to the user to decide whether it is SPDM/CMA in the kernel
> or   the firmware + coco, both are quite possible (it is IDE which is not
> possible without the firmware on AMD but we are not there yet).

The user can control ownership of CMA-SPDM e.g. through a BIOS knob.
And that BIOS knob then influences the outcome of the _OSC negotiation
between platform and OS.


> But the way SPDM is done now is that if the user (as myself) wants to let
> the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
> as CMA is not a (un)loadable module or built-in (with some "blacklist"
> parameters), and does not provide a sysfs knob to control its tentacles.

The problem is every single vendor thinks they can come up with
their own idea of who owns the SPDM session:

I've looked at the Nvidia driver and they've hacked libspdm into it,
so their idea is that the device driver owns the SPDM session.

AMD wants the host to proxy DOE but not own the SPDM session.

We have *standards* for a reason.  So that products are interoperable.

If the kernel tries to accommodate to every vendor's idea of SPDM ownership
we'll end up with an unmaintainable mess of quirks, plus sysfs knobs
which were once intended as a stopgap but can never be removed because
they're userspace ABI.

This needs to be solved in the *specification*.

And the existing solution for who owns a particular PCI feature is _OSC.
Hence this needs to be taken up with the Firmware Working Group at the
PCISIG.


> Note, this PSP firmware is not BIOS (which runs on the same core and has
> same access to PCI as the host OS), it is a separate platform processor
> which only programs IDE keys to the PCI RC (via some some internal bus
> mechanism) but does not do anything on the bus itself and relies on the host
> OS proxying DOE, and there is no APCI between the core and the psp.

Somewhat tangentially, would it be possible in your architecture
that the host or guest asks PSP to program IDE keys into the Root Port?
Or alternatively, access the key registers directly without PSP involvement?

Thanks,

Lukas
Alexey Kardashevskiy Oct. 10, 2023, 12:53 p.m. UTC | #7
On 10/10/23 19:19, Lukas Wunner wrote:
> On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:
>> On 10/10/23 00:49, Lukas Wunner wrote:
>>> PCI Firmware Spec would seem to be appropriate.  However this can't
>>> be solved by the kernel community.
>>
>> How so? It is up to the user to decide whether it is SPDM/CMA in the kernel
>> or   the firmware + coco, both are quite possible (it is IDE which is not
>> possible without the firmware on AMD but we are not there yet).
> 
> The user can control ownership of CMA-SPDM e.g. through a BIOS knob.
> And that BIOS knob then influences the outcome of the _OSC negotiation
> between platform and OS.
> 
> 
>> But the way SPDM is done now is that if the user (as myself) wants to let
>> the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
>> as CMA is not a (un)loadable module or built-in (with some "blacklist"
>> parameters), and does not provide a sysfs knob to control its tentacles.
> 
> The problem is every single vendor thinks they can come up with
> their own idea of who owns the SPDM session:
> 
> I've looked at the Nvidia driver and they've hacked libspdm into it,
> so their idea is that the device driver owns the SPDM session.
 >
> AMD wants the host to proxy DOE but not own the SPDM session.
 >
> We have *standards* for a reason.  So that products are interoperable.

There is no "standard PCI ethernet device", somehow we survive ;)

> If the kernel tries to accommodate to every vendor's idea of SPDM ownership
> we'll end up with an unmaintainable mess of quirks, plus sysfs knobs
> which were once intended as a stopgap but can never be removed because
> they're userspace ABI.

The host kernel needs to accommodate the idea that it is not trusted, 
and neither is the BIOS.

> This needs to be solved in the *specification*.
 >
> And the existing solution for who owns a particular PCI feature is _OSC.
> Hence this needs to be taken up with the Firmware Working Group at the
> PCISIG.

I do like the general idea of specifying things, etc but this place does 
not sound right. The firmware you are talking about has full access to 
PCI, the PSP firmware does not have any (besides the IDE keys 
programming), is there any example of such firmware in the PCI Firmware 
spec? From the BIOS standpoint, the host OS owns DOE and whatever is 
sent over it (on AMD SEV TIO). The host OS chooses not to compose these 
SPDM packets itself (while it could) in order to be able to run guests 
without having them to trust the host OS.

>> Note, this PSP firmware is not BIOS (which runs on the same core and has
>> same access to PCI as the host OS), it is a separate platform processor
>> which only programs IDE keys to the PCI RC (via some some internal bus
>> mechanism) but does not do anything on the bus itself and relies on the host
>> OS proxying DOE, and there is no APCI between the core and the psp.
> 
> Somewhat tangentially, would it be possible in your architecture
> that the host or guest asks PSP to program IDE keys into the Root Port?

Sure it is possible to implement. But this does not help our primary use 
case which is confidential VMs where the host OS is not trusted with the 
keys.

> Or alternatively, access the key registers directly without PSP involvement?

No afaik, for the reason above.


> 
> Thanks,
> 
> Lukas
Jonathan Cameron Oct. 11, 2023, 4:42 p.m. UTC | #8
On Tue, 10 Oct 2023 15:07:41 +1100
Alexey Kardashevskiy <aik@amd.com> wrote:

> On 10/10/23 00:49, Lukas Wunner wrote:
> > On Mon, Oct 09, 2023 at 12:33:35PM +0100, Jonathan Cameron wrote:  
> >> On Sat, 7 Oct 2023 12:04:33 +0200 Lukas Wunner <lukas@wunner.de> wrote:  
> >>> On Fri, Oct 06, 2023 at 09:06:13AM -0700, Dan Williams wrote:  
> >>>> Linux also has an interest in accommodating opt-in to using platform
> >>>> managed keys, so the design requires that key management and session
> >>>> ownership is a system owner policy choice.  
> >>>
> >>> You're pointing out a gap in the specification:
> >>>
> >>> There's an existing mechanism to negotiate which PCI features are
> >>> handled natively by the OS and which by platform firmware and that's
> >>> the _OSC Control Field (PCI Firmware Spec r3.3 table 4-5 and 4-6).
> >>>
> >>> There are currently 10 features whose ownership is negotiated with _OSC,
> >>> examples are Hotplug control and DPC configuration control.
> >>>
> >>> I propose adding an 11th bit to negotiate ownership of the CMA-SPDM
> >>> session.
> >>>
> >>> Once that's added to the PCI Firmware Spec, amending the implementation
> >>> to honor it is trivial:  Just check for platform ownership at the top
> >>> of pci_cma_init() and return.  
> >>
> >> This might want to be a control over the specific DOE instance instead
> >> of a general purpose CMA control (or maybe we want both).
> >>
> >> There is no safe way to access a DOE to find out if it supports CMA
> >> that doesn't potentially break another entity using the mailbox.
> >> Given the DOE instances might be for something entirely different we
> >> can't just decide not to use them at all based on a global control.  
> > 
> > Per PCIe r6.1 sec 6.31.3, the DOE instance used for CMA-SPDM must support
> > "no other data object protocol(s)" besides DOE discovery, CMA-SPDM and
> > Secured CMA-SPDM.
> > 
> > So if the platform doesn't grant the OS control over that DOE instance,
> > unrelated DOE instances and protocols (such as CDAT retrieval) are not
> > affected.
> > 
> > E.g. PCI Firmware Spec r3.3 table 4-5 could be amended with something
> > along the lines of:
> > 
> >    Control Field Bit Offset: 11
> > 
> >    Interpretation: PCI Express Component Measurement and Authentication control
> > 
> >    The operating system sets this bit to 1 to request control over the
> >    DOE instance supporting the CMA-SPDM feature.
> > 
> > You're right that to discover the DOE instance for CMA-SPDM in the
> > first place, it needs to be accessed, which might interfere with the
> > firmware using it.  Perhaps this can be solved with the DOE Busy bit.
> > 
> >   
> >> Any such control becomes messy when hotplug is taken into account.
> >> I suppose we could do a _DSM based on BDF / path to device (to remain
> >> stable across reenumeration) and config space offset to allow the OS
> >> to say 'Hi other entity / firmware are you using this DOE instance?"
> >> Kind of an OSC with parameters.  Also includes the other way around that
> >> the question tells the firmware that if it says "no you can't" the OS
> >> will leave it alone until a reboot or similar - that potentially avoids
> >> the problem that we access DOE instances already without taking care
> >> about this  
> > 
> > PCI Firmware Spec r3.3 table 4-7 lists a number of _DSM Definitions for
> > PCI.  Indeed that could be another solution.  E.g. a newly defined _DSM
> > might return the offset in config space of DOE instance(s) which the OS
> > is not permitted to use.
> > 
> >   
> >> (I dropped ball on this having raised it way back near start
> >> of us adding DOE support.)  
> > 
> > Not your fault.  I think the industry got a bit ahead of itself in
> > its "confidential computing" frenzy and forgot to specify these very
> > basic things.
> > 
> >   
> >> If we do want to do any of these, which spec is appropriate?  Link it to PCI
> >> and propose a PCI firmware spec update? (not sure they have a code
> >> first process available) or make it somewhat generic and propose an
> >> ACPI Code first change?  
> > 
> > PCI Firmware Spec would seem to be appropriate.  However this can't
> > be solved by the kernel community.  
> 
> How so? It is up to the user to decide whether it is SPDM/CMA in the 
> kernel   or   the firmware + coco, both are quite possible (it is IDE 
> which is not possible without the firmware on AMD but we are not there yet).
> 
> But the way SPDM is done now is that if the user (as myself) wants to 
> let the firmware run SPDM - the only choice is disabling CONFIG_CMA 
> completely as CMA is not a (un)loadable module or built-in (with some 
> "blacklist" parameters), and does not provide a sysfs knob to control 
> its tentacles. Kinda harsh.

Not necessarily sufficient unfortunately - if you have a CXL type3 device,
we will run the discovery protocol on the DOE to find out what it supports
(looking for table access protocol used for CDAT). If that hits at wrong point it
will likely break your CMA usage unless you have some hardware lockout of
the relevant PCI config space (in which case that will work with CONFIG_CMA
enabled).

Now you might not care about CXL type 3 devices today, but pretty sure someone
will at somepoint.  Or one of the other uses of DOEs will be relevant.
You might be fine assuming only drivers you've bound ever access the devices
config space, but much nicer to have something standard to ensure that if
we can (and driver specific stuff will deal with it in the short term).

Jonathan

> 
> Note, this PSP firmware is not BIOS (which runs on the same core and has 
> same access to PCI as the host OS), it is a separate platform processor 
> which only programs IDE keys to the PCI RC (via some some internal bus 
> mechanism) but does not do anything on the bus itself and relies on the 
> host OS proxying DOE, and there is no APCI between the core and the psp.
> 
> 
> >  We need to talk to our confidential
> > computing architects and our representatives at the PCISIG to get the
> > spec amended.
> > 
> > Thanks,
> > 
> > Lukas  
>
Jonathan Cameron Oct. 11, 2023, 4:57 p.m. UTC | #9
On Tue, 10 Oct 2023 23:53:16 +1100
Alexey Kardashevskiy <aik@amd.com> wrote:

> On 10/10/23 19:19, Lukas Wunner wrote:
> > On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:  
> >> On 10/10/23 00:49, Lukas Wunner wrote:  
> >>> PCI Firmware Spec would seem to be appropriate.  However this can't
> >>> be solved by the kernel community.  
> >>
> >> How so? It is up to the user to decide whether it is SPDM/CMA in the kernel
> >> or   the firmware + coco, both are quite possible (it is IDE which is not
> >> possible without the firmware on AMD but we are not there yet).  
> > 
> > The user can control ownership of CMA-SPDM e.g. through a BIOS knob.
> > And that BIOS knob then influences the outcome of the _OSC negotiation
> > between platform and OS.
> > 
> >   
> >> But the way SPDM is done now is that if the user (as myself) wants to let
> >> the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
> >> as CMA is not a (un)loadable module or built-in (with some "blacklist"
> >> parameters), and does not provide a sysfs knob to control its tentacles.  
> > 
> > The problem is every single vendor thinks they can come up with
> > their own idea of who owns the SPDM session:
> > 
> > I've looked at the Nvidia driver and they've hacked libspdm into it,
> > so their idea is that the device driver owns the SPDM session.
>  >
> > AMD wants the host to proxy DOE but not own the SPDM session.
>  >
> > We have *standards* for a reason.  So that products are interoperable.  
> 
> There is no "standard PCI ethernet device", somehow we survive ;)
> 
> > If the kernel tries to accommodate to every vendor's idea of SPDM ownership
> > we'll end up with an unmaintainable mess of quirks, plus sysfs knobs
> > which were once intended as a stopgap but can never be removed because
> > they're userspace ABI.  
> 
> The host kernel needs to accommodate the idea that it is not trusted, 
> and neither is the BIOS.
> 
> > This needs to be solved in the *specification*.
>  >
> > And the existing solution for who owns a particular PCI feature is _OSC.
> > Hence this needs to be taken up with the Firmware Working Group at the
> > PCISIG.  
> 
> I do like the general idea of specifying things, etc but this place does 
> not sound right. The firmware you are talking about has full access to 
> PCI, the PSP firmware does not have any (besides the IDE keys 
> programming), is there any example of such firmware in the PCI Firmware 
> spec? From the BIOS standpoint, the host OS owns DOE and whatever is 
> sent over it (on AMD SEV TIO). The host OS chooses not to compose these 
> SPDM packets itself (while it could) in order to be able to run guests 
> without having them to trust the host OS.

As a minimum I'd like to see something saying - "keep away from discovery
protocol on this DOE instance".  An ACPI _OSC or _DSM or similar could do that.
It won't be needed for every approach, but it might for some.

Then either firmwware knows what to do, or a specific driver does.

If your proxy comes up late enough that we've already done (and cached) discovery
protocols results then this might not be a problem for this particular
approach as we have no reason to rerun discovery (other than hotplug in which
case there is lots of other stuff to do anyway).

For your case we need some hooks for the PSP to be able to drive the SPDM session
but that should be easy to allow. I don't think precludes the hypervisor also
verifying the hardware is trusted by it along the way (though not used for IDE).
So if you are relying on a host OS proxy I don't thing you need to disable CONFIG_CMA
(maybe something around resets?)

Potentially the host OS tries first (maybe succeeds - that doesn't matter though
nothing wrong with defense in depth) and then the PSP via a proxy does it all over
again which is fine.  All we need to do is guarantee ordering and I think we are
fine for that.

Far too many possible models here but such is life I guess.

> 
> >> Note, this PSP firmware is not BIOS (which runs on the same core and has
> >> same access to PCI as the host OS), it is a separate platform processor
> >> which only programs IDE keys to the PCI RC (via some some internal bus
> >> mechanism) but does not do anything on the bus itself and relies on the host
> >> OS proxying DOE, and there is no APCI between the core and the psp.  
> > 
> > Somewhat tangentially, would it be possible in your architecture
> > that the host or guest asks PSP to program IDE keys into the Root Port?  
> 
> Sure it is possible to implement. But this does not help our primary use 
> case which is confidential VMs where the host OS is not trusted with the 
> keys.
> 
> > Or alternatively, access the key registers directly without PSP involvement?  
> 
> No afaik, for the reason above.
> 
> 
> > 
> > Thanks,
> > 
> > Lukas  
>
Alexey Kardashevskiy Oct. 12, 2023, 3 a.m. UTC | #10
On 12/10/23 03:57, Jonathan Cameron wrote:
> On Tue, 10 Oct 2023 23:53:16 +1100
> Alexey Kardashevskiy <aik@amd.com> wrote:
> 
>> On 10/10/23 19:19, Lukas Wunner wrote:
>>> On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:
>>>> On 10/10/23 00:49, Lukas Wunner wrote:
>>>>> PCI Firmware Spec would seem to be appropriate.  However this can't
>>>>> be solved by the kernel community.
>>>>
>>>> How so? It is up to the user to decide whether it is SPDM/CMA in the kernel
>>>> or   the firmware + coco, both are quite possible (it is IDE which is not
>>>> possible without the firmware on AMD but we are not there yet).
>>>
>>> The user can control ownership of CMA-SPDM e.g. through a BIOS knob.
>>> And that BIOS knob then influences the outcome of the _OSC negotiation
>>> between platform and OS.
>>>
>>>    
>>>> But the way SPDM is done now is that if the user (as myself) wants to let
>>>> the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
>>>> as CMA is not a (un)loadable module or built-in (with some "blacklist"
>>>> parameters), and does not provide a sysfs knob to control its tentacles.
>>>
>>> The problem is every single vendor thinks they can come up with
>>> their own idea of who owns the SPDM session:
>>>
>>> I've looked at the Nvidia driver and they've hacked libspdm into it,
>>> so their idea is that the device driver owns the SPDM session.
>>   >
>>> AMD wants the host to proxy DOE but not own the SPDM session.
>>   >
>>> We have *standards* for a reason.  So that products are interoperable.
>>
>> There is no "standard PCI ethernet device", somehow we survive ;)
>>
>>> If the kernel tries to accommodate to every vendor's idea of SPDM ownership
>>> we'll end up with an unmaintainable mess of quirks, plus sysfs knobs
>>> which were once intended as a stopgap but can never be removed because
>>> they're userspace ABI.
>>
>> The host kernel needs to accommodate the idea that it is not trusted,
>> and neither is the BIOS.
>>
>>> This needs to be solved in the *specification*.
>>   >
>>> And the existing solution for who owns a particular PCI feature is _OSC.
>>> Hence this needs to be taken up with the Firmware Working Group at the
>>> PCISIG.
>>
>> I do like the general idea of specifying things, etc but this place does
>> not sound right. The firmware you are talking about has full access to
>> PCI, the PSP firmware does not have any (besides the IDE keys
>> programming), is there any example of such firmware in the PCI Firmware
>> spec? From the BIOS standpoint, the host OS owns DOE and whatever is
>> sent over it (on AMD SEV TIO). The host OS chooses not to compose these
>> SPDM packets itself (while it could) in order to be able to run guests
>> without having them to trust the host OS.
> 
> As a minimum I'd like to see something saying - "keep away from discovery
> protocol on this DOE instance".  An ACPI _OSC or _DSM or similar could do that.
> It won't be needed for every approach, but it might for some.

I am relying on the existing DOE code to do the discovery. No APCI in 
the SEV TIO picture.

> Then either firmwware knows what to do, or a specific driver does.
> 
> If your proxy comes up late enough that we've already done (and cached) discovery
> protocols results then this might not be a problem for this particular
> approach as we have no reason to rerun discovery (other than hotplug in which
> case there is lots of other stuff to do anyway).
> 
> For your case we need some hooks for the PSP to be able to drive the SPDM session
> but that should be easy to allow. 

This is just a couple of calls:
doe_md = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG, 
PCI_DOE_PROTOCOL_SECURED_CMA_SPDM);
and
pci_doe(doe_mb, PCI_VENDOR_ID_PCI_SIG, 
PCI_DOE_PROTOCOL_SECURED_CMA_SPDM, ...)


> I don't think precludes the hypervisor also
> verifying the hardware is trusted by it along the way (though not used for IDE).
> So if you are relying on a host OS proxy I don't thing you need to disable CONFIG_CMA
> (maybe something around resets?)

If I do the above 2 calls, then pdev->spdm_state will be out of sync.

> Potentially the host OS tries first (maybe succeeds - that doesn't matter though
> nothing wrong with defense in depth) and then the PSP via a proxy does it all over
> again which is fine.  All we need to do is guarantee ordering and I think we are
> fine for that.

Only trusted bits go all over again, untrusted stuff such as discovery 
is still done by the host OS and PSP is not rerunning it.


> Far too many possible models here but such is life I guess.

True. When I joined the x86 world (quite recently), I was surprised how 
different AMD and Intel are in everything besides the userspace :)


>>>> Note, this PSP firmware is not BIOS (which runs on the same core and has
>>>> same access to PCI as the host OS), it is a separate platform processor
>>>> which only programs IDE keys to the PCI RC (via some some internal bus
>>>> mechanism) but does not do anything on the bus itself and relies on the host
>>>> OS proxying DOE, and there is no APCI between the core and the psp.
>>>
>>> Somewhat tangentially, would it be possible in your architecture
>>> that the host or guest asks PSP to program IDE keys into the Root Port?
>>
>> Sure it is possible to implement. But this does not help our primary use
>> case which is confidential VMs where the host OS is not trusted with the
>> keys.
>>
>>> Or alternatively, access the key registers directly without PSP involvement?
>>
>> No afaik, for the reason above.
Lukas Wunner Oct. 12, 2023, 9:15 a.m. UTC | #11
On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:
> But the way SPDM is done now is that if the user (as myself) wants to let
> the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
> as CMA is not a (un)loadable module or built-in (with some "blacklist"
> parameters), and does not provide a sysfs knob to control its tentacles.
> Kinda harsh.

On AMD SEV-TIO, does the PSP perform SPDM exchanges with a device
*before* it is passed through to a guest?  If so, why does it do that?

Dan and I discussed this off-list and Dan is arguing for lazy attestation,
i.e. the TSM should only have the need to perform SPDM exchanges with
the device when it is passed through.

So the host enumerates the DOE protocols and authenticates the device.
When the device is passed through, patch 12/12 ensures that the host
keeps its hands off of the device, thus affording the TSM exclusive
SPDM control.

I agree that the commit message of 12/12 is utterly misleading in that
it says "the guest" is granted exclusive control.  It should say "the TSM"
instead.  (There might be implementations where the guest itself has
the role of the TSM and authenticates the device on its own behalf,
but PCIe r6.1 sec 11 uses the term "TSM" so that's what the commit
message needs to use.)

However apart from the necessary rewrite of the commit message and
perhaps a rename of the PCI_CMA_OWNED_BY_GUEST flag, I think patch 12/12
should already be doing exactly what you need -- provided that the
PSP doesn't perform SPDM exchanges before passthrough.  If it already
performs them, say, on boot, I'd like to understand the reason.

Thanks,

Lukas
Alexey Kardashevskiy Oct. 12, 2023, 11:18 a.m. UTC | #12
On 12/10/23 20:15, Lukas Wunner wrote:
> On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:
>> But the way SPDM is done now is that if the user (as myself) wants to let
>> the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
>> as CMA is not a (un)loadable module or built-in (with some "blacklist"
>> parameters), and does not provide a sysfs knob to control its tentacles.
>> Kinda harsh.
> 
> On AMD SEV-TIO, does the PSP perform SPDM exchanges with a device
> *before* it is passed through to a guest?  If so, why does it do that?

Yes, to set up IDE. SEV TIO is designed in a way that there is one 
stream == set of keys per the PF's traffic class.

It is like this - imagine a TDISP+SRIOV device with hundreds VFs passed 
through to hundreds VMs. The host still owns the PF, provides DOE for 
the PSP, the PSP owns a handful of keys (one will do really, I have not 
fully grasped the idea when one would want traffic classes but ok, up to 
8), and hundreds VFs work using this few (or one) keys, and the PF works 
as well, just cannot know the IDE key (==cannot spy on VFs via something 
like PCI bridge/retimer or logic analyzer). It is different than what 
you are doing, DOE is the only common thing so far (or ever?).

btw the PSP is not able to initiate SPDM traffic by itself, when the 
host decides it wants to setup IDE (via a PSP in SEV TIO), it talks to 
the PSP which can return "I want to talk to the device, here are 
req/resp buffers", in a loop, until the PSP returns something else.

> Dan and I discussed this off-list and Dan is arguing for lazy attestation,
> i.e. the TSM should only have the need to perform SPDM exchanges with
> the device when it is passed through.

Well, I'd expect that in most cases VF is going to be passed through and 
IDE setup is done via PF which won't be passed through in such cases as 
it has to manage VFs.

> So the host enumerates the DOE protocols

Yes.

> and authenticates the device.

No objection here. But PSP will need to rerun this, but still via the 
host's DOE.

> When the device is passed through, patch 12/12 ensures that the host
> keeps its hands off of the device, thus affording the TSM exclusive
> SPDM control.

If a PF is passed through - I guess yes we could use that, but how is 
this going to work for a VF?

> I agree that the commit message of 12/12 is utterly misleading in that
> it says "the guest" is granted exclusive control.  It should say "the TSM"
> instead.  (There might be implementations where the guest itself has
> the role of the TSM and authenticates the device on its own behalf,
> but PCIe r6.1 sec 11 uses the term "TSM" so that's what the commit
> message needs to use.)

This should work as long as DOE is still available (as of today).

> However apart from the necessary rewrite of the commit message and
> perhaps a rename of the PCI_CMA_OWNED_BY_GUEST flag, I think patch 12/12
> should already be doing exactly what you need -- provided that the
> PSP doesn't perform SPDM exchanges before passthrough.  If it already
> performs them, say, on boot, I'd like to understand the reason.

In out design this does not have to happen on the host's boot. But I 
wonder if some PF host driver authenticated some device and then we 
create a bunch of VFs and pass the SPDM ownership of the PF to the PSP 
to reauthentificate it again - the already running PF host driver may 
become upset, may it? 12/12 assumes the host driver is VFIO-PCI but it 
won't be, VFs will be bound to VFIO-PCI. Hope this all makes sense. Thanks,


> 
> Thanks,
> 
> Lukas
Samuel Ortiz Oct. 12, 2023, 1:13 p.m. UTC | #13
On Thu, Oct 12, 2023 at 11:15:42AM +0200, Lukas Wunner wrote:
> On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:
> > But the way SPDM is done now is that if the user (as myself) wants to let
> > the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
> > as CMA is not a (un)loadable module or built-in (with some "blacklist"
> > parameters), and does not provide a sysfs knob to control its tentacles.
> > Kinda harsh.
> 
> On AMD SEV-TIO, does the PSP perform SPDM exchanges with a device
> *before* it is passed through to a guest?  If so, why does it do that?

SPDM exchanges would be done with the DSM, i.e. through the PF, which is
typically *not* passed through to guests. VFs are.

The RISC-V CoVE-IO [1] spec follows similar flows as SEV-TIO (and to
some extend TDX-Connect) and expects the host to explicitly request the
TSM to establish an SPDM connection with the DSM (PF) before passing one
VF through a TSM managed guest. VFs would be vfio bound, not the PF, so
I think patch #12 does not solve our problem here. 

> Dan and I discussed this off-list and Dan is arguing for lazy attestation,
> i.e. the TSM should only have the need to perform SPDM exchanges with
> the device when it is passed through.
> 
> So the host enumerates the DOE protocols and authenticates the device.
> When the device is passed through, patch 12/12 ensures that the host
> keeps its hands off of the device, thus affording the TSM exclusive
> SPDM control.

Just to re-iterate: The TSM does not talk SPDM with the passed
through device(s), but with the corresponding PF. If the host kernel
owns the SPDM connection when the TSM initiates the SPDM connection with
the DSM (For IDE key setup), the connection establishment will fail.
Both CoVE-IO and SEV-TIO (Alexey, please correct me if I'm wrong)
expect the host to explicitly ask the TSM to establish that SPDM
connection. That request should somehow come from KVM, which then would
have to destroy the existing CMA/SPDM connection in order to give the
TSM a chance to successfully establish the SPDM link.

Cheers,
Samuel.

[1] https://github.com/riscv-non-isa/riscv-ap-tee-io/blob/main/specification/07-theory_operations.adoc
>
Jonathan Cameron Oct. 12, 2023, 3:15 p.m. UTC | #14
On Thu, 12 Oct 2023 14:00:00 +1100
Alexey Kardashevskiy <aik@amd.com> wrote:

> On 12/10/23 03:57, Jonathan Cameron wrote:
> > On Tue, 10 Oct 2023 23:53:16 +1100
> > Alexey Kardashevskiy <aik@amd.com> wrote:
> >   
> >> On 10/10/23 19:19, Lukas Wunner wrote:  
> >>> On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:  
> >>>> On 10/10/23 00:49, Lukas Wunner wrote:  
> >>>>> PCI Firmware Spec would seem to be appropriate.  However this can't
> >>>>> be solved by the kernel community.  
> >>>>
> >>>> How so? It is up to the user to decide whether it is SPDM/CMA in the kernel
> >>>> or   the firmware + coco, both are quite possible (it is IDE which is not
> >>>> possible without the firmware on AMD but we are not there yet).  
> >>>
> >>> The user can control ownership of CMA-SPDM e.g. through a BIOS knob.
> >>> And that BIOS knob then influences the outcome of the _OSC negotiation
> >>> between platform and OS.
> >>>
> >>>      
> >>>> But the way SPDM is done now is that if the user (as myself) wants to let
> >>>> the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
> >>>> as CMA is not a (un)loadable module or built-in (with some "blacklist"
> >>>> parameters), and does not provide a sysfs knob to control its tentacles.  
> >>>
> >>> The problem is every single vendor thinks they can come up with
> >>> their own idea of who owns the SPDM session:
> >>>
> >>> I've looked at the Nvidia driver and they've hacked libspdm into it,
> >>> so their idea is that the device driver owns the SPDM session.
> >>   >
> >>> AMD wants the host to proxy DOE but not own the SPDM session.
> >>   >
> >>> We have *standards* for a reason.  So that products are interoperable.  
> >>
> >> There is no "standard PCI ethernet device", somehow we survive ;)
> >>  
> >>> If the kernel tries to accommodate to every vendor's idea of SPDM ownership
> >>> we'll end up with an unmaintainable mess of quirks, plus sysfs knobs
> >>> which were once intended as a stopgap but can never be removed because
> >>> they're userspace ABI.  
> >>
> >> The host kernel needs to accommodate the idea that it is not trusted,
> >> and neither is the BIOS.
> >>  
> >>> This needs to be solved in the *specification*.
> >>   >
> >>> And the existing solution for who owns a particular PCI feature is _OSC.
> >>> Hence this needs to be taken up with the Firmware Working Group at the
> >>> PCISIG.  
> >>
> >> I do like the general idea of specifying things, etc but this place does
> >> not sound right. The firmware you are talking about has full access to
> >> PCI, the PSP firmware does not have any (besides the IDE keys
> >> programming), is there any example of such firmware in the PCI Firmware
> >> spec? From the BIOS standpoint, the host OS owns DOE and whatever is
> >> sent over it (on AMD SEV TIO). The host OS chooses not to compose these
> >> SPDM packets itself (while it could) in order to be able to run guests
> >> without having them to trust the host OS.  
> > 
> > As a minimum I'd like to see something saying - "keep away from discovery
> > protocol on this DOE instance".  An ACPI _OSC or _DSM or similar could do that.
> > It won't be needed for every approach, but it might for some.  
> 
> I am relying on the existing DOE code to do the discovery. No APCI in 
> the SEV TIO picture.
> 
> > Then either firmwware knows what to do, or a specific driver does.
> > 
> > If your proxy comes up late enough that we've already done (and cached) discovery
> > protocols results then this might not be a problem for this particular
> > approach as we have no reason to rerun discovery (other than hotplug in which
> > case there is lots of other stuff to do anyway).
> > 
> > For your case we need some hooks for the PSP to be able to drive the SPDM session
> > but that should be easy to allow.   
> 
> This is just a couple of calls:
> doe_md = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG, 
> PCI_DOE_PROTOCOL_SECURED_CMA_SPDM);
> and
> pci_doe(doe_mb, PCI_VENDOR_ID_PCI_SIG, 
> PCI_DOE_PROTOCOL_SECURED_CMA_SPDM, ...)
> 
> 
> > I don't think precludes the hypervisor also
> > verifying the hardware is trusted by it along the way (though not used for IDE).
> > So if you are relying on a host OS proxy I don't thing you need to disable CONFIG_CMA
> > (maybe something around resets?)  
> 
> If I do the above 2 calls, then pdev->spdm_state will be out of sync.

Understood - might need a hand-off function call.  Or put something
in the pci_find_doe_mailbox() - though currently we don't have a way to release
it again.

Or alternatively we might not care that it's out of sync. The host core code
doesn't need to know if you separately created an SPDM session.
Might just be a comment saying that variable is only indicating the state
of the host kernel managed spdm session - there might be others.

> 
> > Potentially the host OS tries first (maybe succeeds - that doesn't matter though
> > nothing wrong with defense in depth) and then the PSP via a proxy does it all over
> > again which is fine.  All we need to do is guarantee ordering and I think we are
> > fine for that.  
> 
> Only trusted bits go all over again, untrusted stuff such as discovery 
> is still done by the host OS and PSP is not rerunning it.
> 
> 
> > Far too many possible models here but such is life I guess.  
> 
> True. When I joined the x86 world (quite recently), I was surprised how 
> different AMD and Intel are in everything besides the userspace :)

:)

> 
> 
> >>>> Note, this PSP firmware is not BIOS (which runs on the same core and has
> >>>> same access to PCI as the host OS), it is a separate platform processor
> >>>> which only programs IDE keys to the PCI RC (via some some internal bus
> >>>> mechanism) but does not do anything on the bus itself and relies on the host
> >>>> OS proxying DOE, and there is no APCI between the core and the psp.  
> >>>
> >>> Somewhat tangentially, would it be possible in your architecture
> >>> that the host or guest asks PSP to program IDE keys into the Root Port?  
> >>
> >> Sure it is possible to implement. But this does not help our primary use
> >> case which is confidential VMs where the host OS is not trusted with the
> >> keys.
> >>  
> >>> Or alternatively, access the key registers directly without PSP involvement?  
> >>
> >> No afaik, for the reason above.  
> 
>
Jonathan Cameron Oct. 12, 2023, 3:25 p.m. UTC | #15
On Thu, 12 Oct 2023 22:18:27 +1100
Alexey Kardashevskiy <aik@amd.com> wrote:

> On 12/10/23 20:15, Lukas Wunner wrote:
> > On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:  
> >> But the way SPDM is done now is that if the user (as myself) wants to let
> >> the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
> >> as CMA is not a (un)loadable module or built-in (with some "blacklist"
> >> parameters), and does not provide a sysfs knob to control its tentacles.
> >> Kinda harsh.  
> > 
> > On AMD SEV-TIO, does the PSP perform SPDM exchanges with a device
> > *before* it is passed through to a guest?  If so, why does it do that?  
> 
> Yes, to set up IDE. SEV TIO is designed in a way that there is one 
> stream == set of keys per the PF's traffic class.
> 
> It is like this - imagine a TDISP+SRIOV device with hundreds VFs passed 
> through to hundreds VMs. The host still owns the PF, provides DOE for 
> the PSP, the PSP owns a handful of keys (one will do really, I have not 
> fully grasped the idea when one would want traffic classes but ok, up to 
> 8), and hundreds VFs work using this few (or one) keys, and the PF works 
> as well, just cannot know the IDE key (==cannot spy on VFs via something 
> like PCI bridge/retimer or logic analyzer). It is different than what 
> you are doing, DOE is the only common thing so far (or ever?).
> 
> btw the PSP is not able to initiate SPDM traffic by itself, when the 
> host decides it wants to setup IDE (via a PSP in SEV TIO), it talks to 
> the PSP which can return "I want to talk to the device, here are 
> req/resp buffers", in a loop, until the PSP returns something else.
> 
> > Dan and I discussed this off-list and Dan is arguing for lazy attestation,
> > i.e. the TSM should only have the need to perform SPDM exchanges with
> > the device when it is passed through.  
> 
> Well, I'd expect that in most cases VF is going to be passed through and 
> IDE setup is done via PF which won't be passed through in such cases as 
> it has to manage VFs.
> 
> > So the host enumerates the DOE protocols  
> 
> Yes.
> 
> > and authenticates the device.  
> 
> No objection here. But PSP will need to rerun this, but still via the 
> host's DOE.
> 
> > When the device is passed through, patch 12/12 ensures that the host
> > keeps its hands off of the device, thus affording the TSM exclusive
> > SPDM control.  
> 
> If a PF is passed through - I guess yes we could use that, but how is 
> this going to work for a VF?
> 
> > I agree that the commit message of 12/12 is utterly misleading in that
> > it says "the guest" is granted exclusive control.  It should say "the TSM"
> > instead.  (There might be implementations where the guest itself has
> > the role of the TSM and authenticates the device on its own behalf,
> > but PCIe r6.1 sec 11 uses the term "TSM" so that's what the commit
> > message needs to use.)  
> 
> This should work as long as DOE is still available (as of today).
> 
> > However apart from the necessary rewrite of the commit message and
> > perhaps a rename of the PCI_CMA_OWNED_BY_GUEST flag, I think patch 12/12
> > should already be doing exactly what you need -- provided that the
> > PSP doesn't perform SPDM exchanges before passthrough.  If it already
> > performs them, say, on boot, I'd like to understand the reason.  
> 
> In out design this does not have to happen on the host's boot. But I 
> wonder if some PF host driver authenticated some device and then we 
> create a bunch of VFs and pass the SPDM ownership of the PF to the PSP 
> to reauthentificate it again - the already running PF host driver may 
> become upset, may it? 12/12 assumes the host driver is VFIO-PCI but it 
> won't be, VFs will be bound to VFIO-PCI. Hope this all makes sense. Thanks,

Without some experiments with real drivers, will be hard to be sure, but
I'd expect it to be fine as the host driver bound after attestation (or
what's the point?)
In this patch set attestation only happens again on a reset or kicking it
because of new certs.  For reset, your PSP should be doing it all over again
anyway so that can happen after the host driver has dealt with the reset.
For the manual poking to retry attestation, if the model is we don't
load the driver until the attestation succeeds then that should be fine
(as driver not loaded).

The lock out needed for PF pass through doesn't apply given we are poking
it from the PSP via the host.

So I think patch 12 is irrelevant to your usecase rather than a problem.

May well be dragons in the corner cases.  If we need a lockout for
after the PSP gets involved, then fair enough.

Jonathan

> 
> 
> > 
> > Thanks,
> > 
> > Lukas  
>
Jonathan Cameron Oct. 12, 2023, 3:32 p.m. UTC | #16
On Thu, 12 Oct 2023 15:13:31 +0200
Samuel Ortiz <sameo@rivosinc.com> wrote:

> On Thu, Oct 12, 2023 at 11:15:42AM +0200, Lukas Wunner wrote:
> > On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:  
> > > But the way SPDM is done now is that if the user (as myself) wants to let
> > > the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
> > > as CMA is not a (un)loadable module or built-in (with some "blacklist"
> > > parameters), and does not provide a sysfs knob to control its tentacles.
> > > Kinda harsh.  
> > 
> > On AMD SEV-TIO, does the PSP perform SPDM exchanges with a device
> > *before* it is passed through to a guest?  If so, why does it do that?  
> 
> SPDM exchanges would be done with the DSM, i.e. through the PF, which is
> typically *not* passed through to guests. VFs are.
> 
> The RISC-V CoVE-IO [1] spec follows similar flows as SEV-TIO (and to
> some extend TDX-Connect) and expects the host to explicitly request the
> TSM to establish an SPDM connection with the DSM (PF) before passing one
> VF through a TSM managed guest. VFs would be vfio bound, not the PF, so
> I think patch #12 does not solve our problem here. 
> 
> > Dan and I discussed this off-list and Dan is arguing for lazy attestation,
> > i.e. the TSM should only have the need to perform SPDM exchanges with
> > the device when it is passed through.
> > 
> > So the host enumerates the DOE protocols and authenticates the device.
> > When the device is passed through, patch 12/12 ensures that the host
> > keeps its hands off of the device, thus affording the TSM exclusive
> > SPDM control.  
> 
> Just to re-iterate: The TSM does not talk SPDM with the passed
> through device(s), but with the corresponding PF. If the host kernel
> owns the SPDM connection when the TSM initiates the SPDM connection with
> the DSM (For IDE key setup), the connection establishment will fail.
> Both CoVE-IO and SEV-TIO (Alexey, please correct me if I'm wrong)
> expect the host to explicitly ask the TSM to establish that SPDM
> connection. That request should somehow come from KVM, which then would
> have to destroy the existing CMA/SPDM connection in order to give the
> TSM a chance to successfully establish the SPDM link.

Agreed - I don't see a problem with throwing away the initial connection.
In these cases you are passing that role on to another entity - the
job of this patch set is done.

I'm not clear yet if we need an explicit lock out similar to the VFIO
one for PF pass through or if everything will happen in a 'safe' order
anyway. I suspect a lockout on the ability to re attest is necessary
if the PF driver is loaded.

Perhaps just dropping the
+#if IS_ENABLED(CONFIG_VFIO_PCI_CORE)
and letting other PF drivers or another bit of core kernel code
(I'm not sure where the proxy resides for the models being discussed)
claim ownership is enough?

Jonathan

> 
> Cheers,
> Samuel.
> 
> [1] https://github.com/riscv-non-isa/riscv-ap-tee-io/blob/main/specification/07-theory_operations.adoc
> >   
>
Samuel Ortiz Oct. 13, 2023, 5:03 a.m. UTC | #17
On Thu, Oct 12, 2023 at 04:32:21PM +0100, Jonathan Cameron wrote:
> On Thu, 12 Oct 2023 15:13:31 +0200
> Samuel Ortiz <sameo@rivosinc.com> wrote:
> 
> > On Thu, Oct 12, 2023 at 11:15:42AM +0200, Lukas Wunner wrote:
> > > On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:  
> > > > But the way SPDM is done now is that if the user (as myself) wants to let
> > > > the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
> > > > as CMA is not a (un)loadable module or built-in (with some "blacklist"
> > > > parameters), and does not provide a sysfs knob to control its tentacles.
> > > > Kinda harsh.  
> > > 
> > > On AMD SEV-TIO, does the PSP perform SPDM exchanges with a device
> > > *before* it is passed through to a guest?  If so, why does it do that?  
> > 
> > SPDM exchanges would be done with the DSM, i.e. through the PF, which is
> > typically *not* passed through to guests. VFs are.
> > 
> > The RISC-V CoVE-IO [1] spec follows similar flows as SEV-TIO (and to
> > some extend TDX-Connect) and expects the host to explicitly request the
> > TSM to establish an SPDM connection with the DSM (PF) before passing one
> > VF through a TSM managed guest. VFs would be vfio bound, not the PF, so
> > I think patch #12 does not solve our problem here. 
> > 
> > > Dan and I discussed this off-list and Dan is arguing for lazy attestation,
> > > i.e. the TSM should only have the need to perform SPDM exchanges with
> > > the device when it is passed through.
> > > 
> > > So the host enumerates the DOE protocols and authenticates the device.
> > > When the device is passed through, patch 12/12 ensures that the host
> > > keeps its hands off of the device, thus affording the TSM exclusive
> > > SPDM control.  
> > 
> > Just to re-iterate: The TSM does not talk SPDM with the passed
> > through device(s), but with the corresponding PF. If the host kernel
> > owns the SPDM connection when the TSM initiates the SPDM connection with
> > the DSM (For IDE key setup), the connection establishment will fail.
> > Both CoVE-IO and SEV-TIO (Alexey, please correct me if I'm wrong)
> > expect the host to explicitly ask the TSM to establish that SPDM
> > connection. That request should somehow come from KVM, which then would
> > have to destroy the existing CMA/SPDM connection in order to give the
> > TSM a chance to successfully establish the SPDM link.
> 
> Agreed - I don't see a problem with throwing away the initial connection.
> In these cases you are passing that role on to another entity - the
> job of this patch set is done.

Right. As long as there's a way for the kernel to explicitly drop that
ownership before calling into the TSM for asking it to create a new SPDM
connection, we should be fine. Alexey, would you agree with that
statement?

> I'm not clear yet if we need an explicit lock out similar to the VFIO
> one for PF pass through or if everything will happen in a 'safe' order
> anyway. I suspect a lockout on the ability to re attest is necessary
> if the PF driver is loaded.
>
> Perhaps just dropping the
> +#if IS_ENABLED(CONFIG_VFIO_PCI_CORE)
> and letting other PF drivers or another bit of core kernel code
> (I'm not sure where the proxy resides for the models being discussed)
> claim ownership is enough?

If we agree that other parts of the kernel (I suspect KVM would do the
"Connect to device" call to the TSM) should be able to tear the
established SPDM connection, then yes, the claim/return_ownership() API
should not be only available to VFIO.

Cheers,
Samuel.
Alexey Kardashevskiy Oct. 13, 2023, 11:45 a.m. UTC | #18
On 13/10/23 16:03, Samuel Ortiz wrote:
> On Thu, Oct 12, 2023 at 04:32:21PM +0100, Jonathan Cameron wrote:
>> On Thu, 12 Oct 2023 15:13:31 +0200
>> Samuel Ortiz <sameo@rivosinc.com> wrote:
>>
>>> On Thu, Oct 12, 2023 at 11:15:42AM +0200, Lukas Wunner wrote:
>>>> On Tue, Oct 10, 2023 at 03:07:41PM +1100, Alexey Kardashevskiy wrote:
>>>>> But the way SPDM is done now is that if the user (as myself) wants to let
>>>>> the firmware run SPDM - the only choice is disabling CONFIG_CMA completely
>>>>> as CMA is not a (un)loadable module or built-in (with some "blacklist"
>>>>> parameters), and does not provide a sysfs knob to control its tentacles.
>>>>> Kinda harsh.
>>>>
>>>> On AMD SEV-TIO, does the PSP perform SPDM exchanges with a device
>>>> *before* it is passed through to a guest?  If so, why does it do that?
>>>
>>> SPDM exchanges would be done with the DSM, i.e. through the PF, which is
>>> typically *not* passed through to guests. VFs are.
>>>
>>> The RISC-V CoVE-IO [1] spec follows similar flows as SEV-TIO (and to
>>> some extend TDX-Connect) and expects the host to explicitly request the
>>> TSM to establish an SPDM connection with the DSM (PF) before passing one
>>> VF through a TSM managed guest. VFs would be vfio bound, not the PF, so
>>> I think patch #12 does not solve our problem here.
>>>
>>>> Dan and I discussed this off-list and Dan is arguing for lazy attestation,
>>>> i.e. the TSM should only have the need to perform SPDM exchanges with
>>>> the device when it is passed through.
>>>>
>>>> So the host enumerates the DOE protocols and authenticates the device.
>>>> When the device is passed through, patch 12/12 ensures that the host
>>>> keeps its hands off of the device, thus affording the TSM exclusive
>>>> SPDM control.
>>>
>>> Just to re-iterate: The TSM does not talk SPDM with the passed
>>> through device(s), but with the corresponding PF. If the host kernel
>>> owns the SPDM connection when the TSM initiates the SPDM connection with
>>> the DSM (For IDE key setup), the connection establishment will fail.
>>> Both CoVE-IO and SEV-TIO (Alexey, please correct me if I'm wrong)
>>> expect the host to explicitly ask the TSM to establish that SPDM
>>> connection. That request should somehow come from KVM, which then would
>>> have to destroy the existing CMA/SPDM connection in order to give the
>>> TSM a chance to successfully establish the SPDM link.
>>
>> Agreed - I don't see a problem with throwing away the initial connection.
>> In these cases you are passing that role on to another entity - the
>> job of this patch set is done.
> 
> Right. As long as there's a way for the kernel to explicitly drop that
> ownership before calling into the TSM for asking it to create a new SPDM
> connection, we should be fine. Alexey, would you agree with that
> statement?

Yes, sounds right.

>> I'm not clear yet if we need an explicit lock out similar to the VFIO
>> one for PF pass through or if everything will happen in a 'safe' order
>> anyway. I suspect a lockout on the ability to re attest is necessary
>> if the PF driver is loaded.
>>
>> Perhaps just dropping the
>> +#if IS_ENABLED(CONFIG_VFIO_PCI_CORE)
>> and letting other PF drivers or another bit of core kernel code
>> (I'm not sure where the proxy resides for the models being discussed)
>> claim ownership is enough?
> 
> If we agree that other parts of the kernel (I suspect KVM would do the
> "Connect to device" call to the TSM) should be able to tear the
> established SPDM connection, then yes, the claim/return_ownership() API
> should not be only available to VFIO.

Correct. I just want to make sure that DOE mailboxes stay alive and 
nothing in the host kernel relies on SPDM being still active after 
ownership is transferred to the TSM==PSP.

> 
> Cheers,
> Samuel.