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

[Alistair Francis <Alistair.Francis@wdc.com>]

On Sun, 2024-06-30 at 21:45 +0200, 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
> 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.
> 
> To retain this usage model in the face of authentication and
> encryption,
> CMA-SPDM reauthentication and IDE reestablishment must happen during
> the
> ->resume_noirq phase, before the driver's first access to the device.
> The driver is thus afforded seamless authenticated and encrypted
> access
> until the last moment before suspend and from the first moment after
> resume.
> 
> During the ->resume_noirq phase, device interrupts are not yet
> enabled.
> It is thus impossible to defer CMA-SPDM reauthentication to a user
> space
> component on an attached disk or on the network, making an in-kernel
> SPDM implementation mandatory.
> 
> The same catch-22 exists on recovery from a Conventional Reset:  A
> user
> space SPDM implementation might live on a device which underwent
> reset,
> rendering its execution impossible.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  drivers/pci/cma.c        | 15 +++++++++++++++
>  drivers/pci/pci-driver.c |  1 +
>  drivers/pci/pci.c        | 12 ++++++++++--
>  drivers/pci/pci.h        |  2 ++
>  drivers/pci/pcie/err.c   |  3 +++
>  5 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
> index e974d489c7a2..f2c435b04b92 100644
> --- a/drivers/pci/cma.c
> +++ b/drivers/pci/cma.c
> @@ -195,6 +195,21 @@ void pci_cma_init(struct pci_dev *pdev)
>  	spdm_authenticate(pdev->spdm_state);
>  }
>  
> +/**
> + * pci_cma_reauthenticate() - Perform CMA-SPDM authentication again
> + * @pdev: Device to reauthenticate
> + *
> + * Can be called by drivers after device identity has mutated,
> + * e.g. after downloading firmware to an FPGA device.
> + */
> +void pci_cma_reauthenticate(struct pci_dev *pdev)
> +{
> +	if (!pdev->spdm_state)
> +		return;
> +
> +	spdm_authenticate(pdev->spdm_state);
> +}
> +
>  void pci_cma_destroy(struct pci_dev *pdev)
>  {
>  	if (!pdev->spdm_state)
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index af2996d0d17f..89571f94debc 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -566,6 +566,7 @@ static void pci_pm_default_resume_early(struct
> pci_dev *pci_dev)
>  	pci_pm_power_up_and_verify_state(pci_dev);
>  	pci_restore_state(pci_dev);
>  	pci_pme_restore(pci_dev);
> +	pci_cma_reauthenticate(pci_dev);
>  }
>  
>  static void pci_pm_bridge_power_up_actions(struct pci_dev *pci_dev)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 59e0949fb079..2a8063e7f2e0 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4980,8 +4980,16 @@ static int pci_reset_bus_function(struct
> pci_dev *dev, bool probe)
>  
>  	rc = pci_dev_reset_slot_function(dev, probe);
>  	if (rc != -ENOTTY)
> -		return rc;
> -	return pci_parent_bus_reset(dev, probe);
> +		goto done;
> +
> +	rc = pci_parent_bus_reset(dev, probe);
> +
> +done:
> +	/* CMA-SPDM state is lost upon a Conventional Reset */
> +	if (!probe)
> +		pci_cma_reauthenticate(dev);
> +
> +	return rc;
>  }
>  
>  static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index fc90845caf83..b4c2ce5fd070 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -336,9 +336,11 @@ static inline void pci_doe_disconnected(struct
> pci_dev *pdev) { }
>  #ifdef CONFIG_PCI_CMA
>  void pci_cma_init(struct pci_dev *pdev);
>  void pci_cma_destroy(struct pci_dev *pdev);
> +void pci_cma_reauthenticate(struct pci_dev *pdev);
>  #else
>  static inline void pci_cma_init(struct pci_dev *pdev) { }
>  static inline void pci_cma_destroy(struct pci_dev *pdev) { }
> +static inline void pci_cma_reauthenticate(struct pci_dev *pdev) { }
>  #endif
>  
>  /**
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index 31090770fffc..0028582f0590 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -133,6 +133,9 @@ static int report_slot_reset(struct pci_dev *dev,
> void *data)
>  	pci_ers_result_t vote, *result = data;
>  	const struct pci_error_handlers *err_handler;
>  
> +	/* CMA-SPDM state is lost upon a Conventional Reset */
> +	pci_cma_reauthenticate(dev);
> +
>  	device_lock(&dev->dev);
>  	pdrv = dev->driver;
>  	if (!pdrv || !pdrv->err_handler || !pdrv->err_handler-
> >slot_reset)