Re: [PATCH 08/11] PCI/IDE: Add IDE establishment helpers

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

Alexey Kardashevskiy wrote:
> 
> 
> On 6/12/24 09:24, 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().
> > 
> > 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
> > 
> > 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.
> > 
> > Thanks to Wu Hao for a draft implementation of this infrastructure.
> > 
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Lukas Wunner <lukas@wunner.de>
> > Cc: Samuel Ortiz <sameo@rivosinc.com>
> > Co-developed-by: Alexey Kardashevskiy <aik@amd.com>
> > Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> > Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
> > Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >   .../ABI/testing/sysfs-devices-pci-host-bridge      |   28 +++
> >   drivers/pci/ide.c                                  |  192 ++++++++++++++++++++
> >   drivers/pci/pci.h                                  |    4
> >   drivers/pci/probe.c                                |    1
> >   include/linux/pci-ide.h                            |   33 +++
> >   include/linux/pci.h                                |    4
> >   6 files changed, 262 insertions(+)
> >   create mode 100644 Documentation/ABI/testing/sysfs-devices-pci-host-bridge
> >   create mode 100644 include/linux/pci-ide.h
> > 
[..]
> > diff --git a/drivers/pci/ide.c b/drivers/pci/ide.c
> > index a0c09d9e0b75..c37f35f0d2c0 100644
> > --- a/drivers/pci/ide.c
> > +++ b/drivers/pci/ide.c
[..]
> > +static void __pci_ide_stream_setup(struct pci_dev *pdev, struct pci_ide *ide)
> > +{
> > +	int pos;
> > +	u32 val;
> > +
> > +	pos = sel_ide_offset(pdev->sel_ide_cap, ide->stream_id,
> > +			     pdev->nr_ide_mem);
> > +
> > +	val = FIELD_PREP(PCI_IDE_SEL_RID_1_LIMIT_MASK, ide->devid_end);
> > +	pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_1, val);
> > +
> > +	val = FIELD_PREP(PCI_IDE_SEL_RID_2_VALID, 1) |
> > +	      FIELD_PREP(PCI_IDE_SEL_RID_2_BASE_MASK, ide->devid_start) |
> > +	      FIELD_PREP(PCI_IDE_SEL_RID_2_SEG_MASK, ide->domain);
> > +	pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_2, val);
> > +
> > +	for (int i = 0; i < ide->nr_mem; i++) {
> 
> 
> This needs to test that (pdev->nr_ide_mem >= ide->nr_mem), easy to miss 
> especially when PCI_IDE_SETUP_ROOT_PORT. Thanks,

Good catch.

I think the more appropriate place to enforce this is in
pci_ide_stream_probe() with something like the below... unless I am
mistaken and address association settings do not need to be identical
between endpoint and Root Port?

@@ -99,9 +99,13 @@ void pci_init_host_bridge_ide(struct pci_host_bridge *hb)
  */
 void pci_ide_stream_probe(struct pci_dev *pdev, struct pci_ide *ide)
 {
+       struct pci_dev *rp = pcie_find_root_port(pdev);
        int num_vf = pci_num_vf(pdev);
 
-       *ide = (struct pci_ide) { .stream_id = -1 };
+       *ide = (struct pci_ide) {
+               .stream_id = -1,
+               .nr_mem = min(pdev->nr_ide_mem, rp->nr_ide_mem),
+       };
 
        /* PCIe Requester ID == Linux "devid" */
        ide->rid_start = pci_dev_id(pdev);