Re: [PATCH 04/11] PCI/IDE: Selective Stream IDE enumeration

[Dan Williams <dan.j.williams@intel.com>]

Alexey Kardashevskiy wrote:
> On 6/12/24 09:23, Dan Williams wrote:
> > Link encryption is a new PCIe capability defined by "PCIe 6.2 section
> > 6.33 Integrity & Data Encryption (IDE)". While it is a standalone port
> > and endpoint capability, it is also a building block for device security
> > defined by "PCIe 6.2 section 11 TEE Device Interface Security Protocol
> > (TDISP)". That protocol coordinates device security setup between the
> > platform TSM (TEE Security Manager) and device DSM (Device Security
> > Manager). While the platform TSM can allocate resources like stream-ids
> > and manage keys, it still requires system software to manage the IDE
> > capability register block.
> > 
> > Add register definitions and basic enumeration for a "selective-stream"
> > IDE capability, a follow on change will select the new CONFIG_PCI_IDE
> > symbol. Note that while the IDE specifications defines both a
> > point-to-point "Link" stream and a root-port-to-endpoint "Selective"
> > stream, only "Selective" is considered for now for platform TSM
> > coordination.
> > 
> > Co-developed-by: Alexey Kardashevskiy <aik@amd.com>
> > Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
[..]
> > +void pci_ide_init(struct pci_dev *pdev)
> > +{
> > +	u16 ide_cap, sel_ide_cap;
> > +	int nr_ide_mem = 0;
> > +	u32 val = 0;
> > +
> > +	if (!pci_is_pcie(pdev))
> > +		return;
> > +
> > +	ide_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_IDE);
> > +	if (!ide_cap)
> > +		return;
> > +
> > +	/*
> > +	 * Check for selective stream capability from endpoint to root-port, and
> > +	 * require consistent number of address association blocks
> > +	 */
> > +	pci_read_config_dword(pdev, ide_cap + PCI_IDE_CAP, &val);
> > +	if ((val & PCI_IDE_CAP_SELECTIVE) == 0)
> > +		return;
> > +
> > +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ENDPOINT) {
> > +		struct pci_dev *rp = pcie_find_root_port(pdev);
> > +
> > +		if (!rp->ide_cap)
> > +			return;
> > +	}
> > +
> > +	if (val & PCI_IDE_CAP_LINK)
> > +		sel_ide_cap = ide_cap + PCI_IDE_LINK_STREAM +
> > +			      (PCI_IDE_CAP_LINK_TC_NUM(val) + 1) *
> > +				      PCI_IDE_LINK_BLOCK_SIZE;
> > +	else
> > +		sel_ide_cap = ide_cap + PCI_IDE_LINK_STREAM;
> > +
> > +	for (int i = 0; i < PCI_IDE_CAP_SELECTIVE_STREAMS_NUM(val); i++) {
> > +		if (i == 0) {
> > +			pci_read_config_dword(pdev, sel_ide_cap, &val);
> > +			nr_ide_mem = PCI_IDE_SEL_CAP_ASSOC_NUM(val);
> > +		} else {
> > +			int offset = sel_ide_offset(sel_ide_cap, i, nr_ide_mem);
> > +
> > +			pci_read_config_dword(pdev, offset, &val);
> > +
> > +			/*
> > +			 * lets not entertain devices that do not have a
> > +			 * constant number of address association blocks
> 
> But why? It is quite easy to support those. Yeah, won't be able to cache 
> nr_ide_mem and will have to read more configspace but a specific 
> selected stream offset can live in pci_ide from 8/11. Thanks,

Specifications often add flexibility without concern for the system
software complexity it implies. I am happy to change it as soon as there
is the first sign of evidence someone might build such a thing, but
otherwise it makes walking this register space simpler. It is not clear
to me that there is an economic reason to build a device with variable
amount of address association per stream block.

Also, apologies for letting this patch set so long, I have finally
cleared some other backlog.