[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250729164504.00000ec2@huawei.com>
Date: Tue, 29 Jul 2025 16:45:04 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Dan Williams <dan.j.williams@...el.com>
CC: <linux-coco@...ts.linux.dev>, <linux-pci@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <bhelgaas@...gle.com>, <aik@....com>,
<lukas@...ner.de>, Samuel Ortiz <sameo@...osinc.com>, Yilun Xu
<yilun.xu@...ux.intel.com>
Subject: Re: [PATCH v4 07/10] PCI/IDE: Add IDE establishment helpers
On Thu, 17 Jul 2025 11:33:55 -0700
Dan Williams <dan.j.williams@...el.com> wrote:
> There are two components to establishing an encrypted link, provisioning
> the stream in Partner Port config-space, and programming the keys into
> the link layer via IDE_KM (IDE Key Management). This new library,
> drivers/pci/ide.c, enables the former. IDE_KM, via a TSM low-level
> driver, is saved for later.
>
> With the platform TSM implementations of SEV-TIO and TDX Connect in mind
> this library abstracts small differences in those implementations. For
> example, TDX Connect handles Root Port register setup while SEV-TIO
> expects System Software to update the Root Port registers. This is the
> rationale for fine-grained 'setup' + 'enable' verbs.
>
> The other design detail for TSM-coordinated IDE establishment is that
> the TSM may manage allocation of Stream IDs, this is why the Stream ID
> value is passed in to pci_ide_stream_setup().
>
> The flow is:
>
> pci_ide_stream_alloc()
> Allocate a Selective IDE Stream Register Block in each Partner Port
> (Endpoint + Root Port), and reserve a host bridge / platform stream
> slot. Gather Partner Port specific stream settings like Requester ID.
> pci_ide_stream_register()
> Publish the stream in sysfs after allocating a Stream ID. In the TSM
> case the TSM allocates the Stream ID for the Partner Port pair.
> pci_ide_stream_setup()
> Program the stream settings to a Partner Port. Caller is responsible
> for optionally calling this for the Root Port as well if the TSM
> implementation requires it.
> pci_ide_stream_enable()
> Try to run the stream after IDE_KM.
>
> In support of system administrators auditing where platform, Root Port,
> and Endpoint IDE stream resources are being spent, the allocated stream
> is reflected as a symlink from the host bridge to the endpoint with the
> name:
>
> stream%d.%d.%d
>
> Where the tuple of integers reflects the allocated platform, Root Port,
> and Endpoint stream index (Selective IDE Stream Register Block) values.
>
> Thanks to Wu Hao for a draft implementation of this infrastructure.
>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: Lukas Wunner <lukas@...ner.de>
> Cc: Samuel Ortiz <sameo@...osinc.com>
> Co-developed-by: Alexey Kardashevskiy <aik@....com>
> Signed-off-by: Alexey Kardashevskiy <aik@....com>
> Co-developed-by: Yilun Xu <yilun.xu@...ux.intel.com>
> Signed-off-by: Yilun Xu <yilun.xu@...ux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
A few minor things inline.
> diff --git a/drivers/pci/ide.c b/drivers/pci/ide.c
> index e15937cdb2a4..cdc773a8b381 100644
> --- a/drivers/pci/ide.c
> +++ b/drivers/pci/ide.c
> @@ -5,6 +5,8 @@
>
> +/**
> + * pci_ide_stream_enable() - try to enable a Selective IDE Stream
> + * @pdev: PCIe device object for either a Root Port or Endpoint Partner Port
> + * @ide: registered and setup IDE settings descriptor
> + *
> + * Activate the stream by writing to the Selective IDE Stream Control
> + * Register, report whether the state successfully transitioned to
> + * secure mode.
and report
> ... Note that the state may go "insecure" at any point after
> + * this check, but that is handled via asynchronous error reporting.
> + */
> +int pci_ide_stream_enable(struct pci_dev *pdev, struct pci_ide *ide)
> +{
> + struct pci_ide_partner *settings = pci_ide_to_settings(pdev, ide);
> + int pos;
> + u32 val;
> +
> + if (!settings)
> + return -ENXIO;
> +
> + pos = sel_ide_offset(pdev, settings);
> +
> + set_ide_sel_ctl(pdev, ide, pos, true);
> +
> + pci_read_config_dword(pdev, pos + PCI_IDE_SEL_STS, &val);
> + if (FIELD_GET(PCI_IDE_SEL_STS_STATE_MASK, val) !=
> + PCI_IDE_SEL_STS_STATE_SECURE) {
> + set_ide_sel_ctl(pdev, ide, pos, false);
> + return -ENXIO;
> + }
> +
> + settings->enable = 1;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_ide_stream_enable);
> diff --git a/include/linux/pci-ide.h b/include/linux/pci-ide.h
> new file mode 100644
> index 000000000000..89c1ef0de841
> --- /dev/null
> +++ b/include/linux/pci-ide.h
> @@ -0,0 +1,70 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
...
> +/**
> + * struct pci_ide_partner - Per port pair Selective IDE Stream settings
> + * @rid_start: Partner Port Requester ID range start
> + * @rid_start: Partner Port Requester ID range end
> + * @stream_index: Selective IDE Stream Register Block selection
> + * @setup: flag to track whether to run pci_ide_stream_teardown for this parnter slot
partner.
> + * @enable: flag whether to run pci_ide_stream_disable for this parnter slot
same again.
> + */
> +struct pci_ide_partner {
> + u16 rid_start;
> + u16 rid_end;
> + u8 stream_index;
> + unsigned int setup:1;
> + unsigned int enable:1;
> +};
> +
> +/**
> + * struct pci_ide - PCIe Selective IDE Stream descriptor
> + * @pdev: PCIe Endpoint in the pci_ide_partner pair
> + * @partner: Per-partner settings
per-partner maybe? Capitalization seems a little random
as mostly you have used them for spec terms, but Per-partner probably
isn't one?
> + * @host_bridge_stream: track platform Stream ID
> + * @stream_id: unique Stream ID (within Partner Port pairing)
> + * @name: name of the established Selective IDE Stream in sysfs
> + *
> + * Negative @stream_id values indicate "uninitialized" on the
> + * expectation that with TSM established IDE the TSM owns the stream_id
> + * allocation.
> + */
> +struct pci_ide {
> + struct pci_dev *pdev;
> + struct pci_ide_partner partner[PCI_IDE_PARTNER_MAX];
> + u8 host_bridge_stream;
> + int stream_id;
> + const char *name;
> +};
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index a7353df51fea..cc83ae274601 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -538,6 +538,8 @@ struct pci_dev {
> u16 ide_cap; /* Link Integrity & Data Encryption */
> u8 nr_ide_mem; /* Address association resources for streams */
> u8 nr_link_ide; /* Link Stream count (Selective Stream offset) */
> + u8 nr_sel_ide; /* Selective Stream count (register block allocator) */
> + DECLARE_BITMAP(ide_stream_map, CONFIG_PCI_IDE_STREAM_MAX);
> unsigned int ide_cfg:1; /* Config cycles over IDE */
> unsigned int ide_tee_limit:1; /* Disallow T=0 traffic over IDE */
> #endif
> @@ -607,6 +609,10 @@ struct pci_host_bridge {
> int domain_nr;
> struct list_head windows; /* resource_entry */
> struct list_head dma_ranges; /* dma ranges resource list */
> +#ifdef CONFIG_PCI_IDE
> + u8 nr_ide_streams; /* Track available vs in-use streams */
Which does it do? Confusing comment.
> + DECLARE_BITMAP(ide_stream_map, CONFIG_PCI_IDE_STREAM_MAX);
> +#endif
Powered by blists - more mailing lists