Re: [RFC PATCH 1/4] PCI/TSM: Create MMIO descriptors via TDISP Report

[Nikolay Borisov]

On 5.10.26 г. 10:02 ч., Ankit Agrawal wrote:
> After pci_tsm_bind() and pci_tsm_lock() the low level TSM driver is
> expected to populate the TDISP GET_DEVICE_INTERFACE_REPORT response
> payload for the device.
>
> Add pci_tsm_mmio_alloc()/pci_tsm_mmio_free() to parse that report into
> a set of encrypted MMIO ranges, and pci_tsm_mmio_setup()/
> pci_tsm_mmio_teardown() to mark those ranges as encrypted in iomem via
> a new encrypted_iomem_resource tree (IORES_DESC_ENCRYPTED). With those
> descriptors the TSM driver can use pci_tsm_mmio_setup() to inform
> ioremap() how to map the device per the device expectations. The VM
> is expected to validate the interface with the relying party before
> accepting the device for operation.
>
> pci_tsm_mmio_alloc() also recovers the obfuscated starting address for
> each encrypted MMIO range, since the VM is never disclosed the HPA
> that correlates to the GPA of the device's MMIO. The obfuscated
> address is BAR aligned.
>
> Based on an original patch by Aneesh Kumar K.V [1], and cherry-picked
> from Dan Williams' upstream commit [2]. Major functional differences
> from Dan's posting is due to the lack of evidence-store infrastructure
> yet (device_evidence / DEVICE_EVIDENCE_TYPE_REPORT). The
> pci_tsm_mmio_alloc() takes the raw report bytes directly from the caller
> instead of pulling them from a device evidence store.
>
> Link: https://lore.kernel.org/linux-coco/20251117140007.122062-8-aneesh.kumar@xxxxxxxxxx/ [1]
> Link: https://lore.kernel.org/all/20260705220819.2472765-15-djbw@xxxxxxxxxx/ [2]
>
> Signed-off-by: Ankit Agrawal <ankita@xxxxxxxxxx>
> Assisted-by: Claude:sonnet-5
> ---
>   drivers/pci/tsm.c       | 236 ++++++++++++++++++++++++++++++++++++++++
>   include/linux/ioport.h  |   2 +
>   include/linux/pci-tsm.h |  33 ++++++
>   kernel/resource.c       |   8 ++
>   4 files changed, 279 insertions(+)
>
> diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> index 5fdcd7f2e820..c8cfe89b6224 100644
> --- a/drivers/pci/tsm.c
> +++ b/drivers/pci/tsm.c
> @@ -9,11 +9,16 @@
>   #define dev_fmt(fmt) "PCI/TSM: " fmt
>   
>   #include <linux/bitfield.h>
> +#include <linux/overflow.h>
>   #include <linux/pci.h>
>   #include <linux/pci-doe.h>
>   #include <linux/pci-tsm.h>
> +#include <linux/range.h>
> +#include <linux/sizes.h>
> +#include <linux/slab.h>
>   #include <linux/sysfs.h>
>   #include <linux/tsm.h>
> +#include <linux/unaligned.h>
>   #include <linux/xarray.h>
>   #include "pci.h"
>   
> @@ -898,3 +903,234 @@ int pci_tsm_doe_transfer(struct pci_dev *pdev, u8 type, const void *req,
>   		       resp, resp_sz);
>   }
>   EXPORT_SYMBOL_GPL(pci_tsm_doe_transfer);
> +
> +static void mmio_teardown(struct pci_tsm_mmio *mmio, int nr)
> +{
> +	while (nr--) {
> +		struct resource *res = pci_tsm_mmio_resource(mmio, nr);
> +
> +		/*
> +		 * Entries past the point where pci_tsm_mmio_setup() stopped
> +		 * (or that were never run through setup() at all, e.g. a
> +		 * caller that only wants the resolved range from
> +		 * pci_tsm_mmio_alloc()) were never insert_resource()'d, so
> +		 * res->parent is NULL. remove_resource() dereferences
> +		 * res->parent unconditionally.
> +		 */
> +		if (res->parent)
> +			remove_resource(res);
> +	}
> +}
> +
> +/**
> + * pci_tsm_mmio_setup() - mark device MMIO as encrypted in iomem
> + * @pdev: device owner of MMIO resources
> + * @mmio: container of an array of resources to mark encrypted
> + */
> +int pci_tsm_mmio_setup(struct pci_dev *pdev, struct pci_tsm_mmio *mmio)
> +{
> +	int i;
> +
> +	device_lock_assert(&pdev->dev);
> +	if (pdev->dev.driver)
> +		return -EBUSY;
> +
> +	for (i = 0; i < mmio->nr; i++) {
> +		struct resource *res = pci_tsm_mmio_resource(mmio, i);
> +		int j;
> +
> +		if (resource_size(res) == 0 || !(res->flags & IORESOURCE_MEM))
> +			break;
> +
> +		/* Only require the caller to set the range, init remainder */
> +		*res = DEFINE_RES_NAMED_DESC(res->start, resource_size(res),
> +					     pci_name(pdev), IORESOURCE_MEM,
> +					     IORES_DESC_ENCRYPTED);
> +
> +		for (j = 0; j < PCI_NUM_RESOURCES; j++)
> +			if (resource_contains(pci_resource_n(pdev, j), res))
> +				break;
> +
> +		/* Request is outside of device MMIO */
> +		if (j >= PCI_NUM_RESOURCES)
> +			break;
> +
> +		if (insert_resource(&encrypted_iomem_resource, res) != 0)
> +			break;
> +	}
> +
> +	if (i >= mmio->nr)
> +		return 0;

Shouldn't this check be if (i == mmio->nr -1 ) i.e all descriptors have 
been initialized, given the for loop condition i can ever be at most 
mmio->nr - 1 i.e the actual index of a descriptor?

<snip>