On Thu, Dec 05, 2024 at 02:24:02PM -0800, Dan Williams wrote:
> There are two components to establishing an encrypted link, provisioning
> the stream in config-space, and programming the keys into the link layer
> via the IDE_KM (key management) protocol. These helpers enable the
> former, and are in support of TSM coordinated IDE_KM. When / if native
> IDE establishment arrives it will share this same config-space
> provisioning flow, but for now IDE_KM, in any form, is saved for a
> follow-on change.
>
> With the TSM implementations of SEV-TIO and TDX Connect in mind this
> abstracts small differences in those implementations. For example, TDX
> Connect handles Root Port registers updates while SEV-TIO expects System
> Software to update the Root Port registers. This is the rationale for
> the PCI_IDE_SETUP_ROOT_PORT flag.
>
> The other design detail for TSM-coordinated IDE establishment is that
> the TSM manages allocation of stream-ids, this is why the stream_id is
> passed in to pci_ide_stream_setup().
s/stream-ids/Stream IDs/ to match spec usage (also several other
places)
> The flow is:
>
> pci_ide_stream_probe()
> Gather stream settings (devid and address filters)
> pci_ide_stream_setup()
> Program the stream settings into the endpoint, and optionally Root Port)
> pci_ide_enable_stream()
> Run the stream after IDE_KM
Not sure what "devid" is. Requester ID? I suppose it's from
PCI_DEVID(), which does turn out to be the PCIe Requester ID. I don't
think the spec uses a "devid" term, so I'd prefer the term used in the
spec.
> In support of system administrators auditing where platform IDE stream
> resources are being spent, the allocated stream is reflected as a
> symlink from the host-bridge to the endpoint.
s/host-bridge/host bridge/ to match typical usage (also elsewhere)
> +++ b/Documentation/ABI/testing/sysfs-devices-pci-host-bridge
> @@ -0,0 +1,28 @@
> +What: /sys/devices/pciDDDDD:BB
> + /sys/devices/.../pciDDDDD:BB
> +Date: December, 2024
> +Contact: linux-pci@xxxxxxxxxxxxxxx
> +Description:
> + A PCI host bridge device parents a PCI bus device topology. PCI
> + controllers may also parent host bridges. The DDDDD:BB format
> + convey the PCI domain number and the bus number for root ports
> + of the host bridge.
"Root ports" doesn't seem quite right here; BB is the root bus number,
and makes sense even for conventional PCI.
I know IDE etc is PCIe-specific, but I think saying "the PCI domain
number and root bus number of the host bridge" would be more accurate
since there can be things other than Root Ports on the root bus, e.g.,
conventional PCI devices, RCiEPs, RCECs, etc.
Typical formatting of domain is %04x.
> +What: pciDDDDD:BB/streamN:DDDDD:BB:DD:F
> +Date: December, 2024
> +Contact: linux-pci@xxxxxxxxxxxxxxx
> +Description:
> + (RO) When a host-bridge has established a secure connection,
> + typically PCIe IDE, between a host-bridge an endpoint, this
> + symlink appears. The primary function is to account how many
> + streams can be returned to the available secure streams pool by
> + invoking the tsm/disconnect flow. The link points to the
> + endpoint PCI device at domain:DDDDD bus:BB device:DD function:F.
s/host-bridge an endpoint/host bridge and an endpoint/
> + * Retrieve stream association parameters for devid (RID) and resources
> + * (device address ranges)
This and other exported functions probably should have kernel-doc.
Document only the implemented parts.
> +void pci_ide_stream_probe(struct pci_dev *pdev, struct pci_ide *ide)
> +void pci_ide_stream_teardown(struct pci_dev *pdev, struct pci_ide *ide,
> + enum pci_ide_flags flags)
> +{
> + struct pci_host_bridge *hb = pci_find_host_bridge(pdev->bus);
> + struct pci_dev *rp = pcie_find_root_port(pdev);
> +
> + __pci_ide_stream_teardown(pdev, ide);
> + if (flags & PCI_IDE_SETUP_ROOT_PORT)
> + __pci_ide_stream_teardown(rp, ide);
Looks like this relies on the caller to supply the same flags as they
previously supplied to pci_ide_stream_setup()? Could/should we
remember the flags to remove the possibility of leaking the RP setup
or trying to teardown RP setup that wasn't done?
> +++ b/include/linux/pci.h
> @@ -601,6 +601,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 /* track IDE stream id allocation */
> + DECLARE_BITMAP(ide_stream_ids, PCI_IDE_SEL_CTL_ID_MAX + 1);
> + struct resource ide_stream_res; /* track ide stream address association */
Seems like overkill to repeat this. Probably remove the comment on
the #ifdef and s/ide/IDE/ here.
