On Mon, Feb 24, 2025 at 12:24:20PM -0800, Dan Williams wrote:
> 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@xxxxxxxxxx>
> > > Cc: Lukas Wunner <lukas@xxxxxxxxx>
> > > Cc: Samuel Ortiz <sameo@xxxxxxxxxxxx>
> > > Co-developed-by: Alexey Kardashevskiy <aik@xxxxxxx>
> > > Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxx>
> > > Co-developed-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> > > ---
> > > .../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),
No, addr associations don't have to be identical.
Now we should fill EP's mem ranges to RP's addr association registers,
so Aik's idea is to check if RP's pdev->nr_ide_mem >= EP's nr_mem.
EP's nr_mem calculation is complicated,
ep_nr_mem = pci_resource_number(PF_pdev, IORESOURCE_MEM) +
pci_resource_number(VF1_pdev, IORESOURCE_MEM) +
pci_resource_number(VF2_pdev, IORESOURCE_MEM) +
...;
It is easily rp->nr_ide_mem < ep_mr_mem, so I don't think check and
fail is a good way, we have to merge ranges. Maybe merging to 1 range
is a simplest solution:
if (rp->nr_ide_mem < ep_nr_mem) {
ide->nr_mem = 1;
/* merge ep ranges and fill ide->mem[0] */
} else {
ide->nr_mem = ep_nr_mem;
/* copy ep ranges to ide->mem[] */
}
A further requirement is, firmware may not agree to use all RP's address
asociation blocks, e.g. Intel TDX Module just use one block. So maybe
add an input parameter:
void pci_ide_stream_probe(struct pci_dev *pdev, int max_ide_nr_mem, struct pci_ide *ide)
{
int nr_ide_mem = min(rp->nr_ide_mem, max_ide_nr_mem);
int ep_nr_mem = calc_ep_nr_mem(pdev);
if (nr_ide_mem < ep_nr_mem) {
ide->nr_mem = 1;
/* merge ep ranges and fill ide->mem[0] */
} else {
ide->nr_mem = ep_nr_mem;
/* copy ep ranges to ide->mem[] */
}
}
BTW: I'm not sure how to fill EP's addr association register, for now I
just skip it and works. Maybe my device doesn't care about them at all.
Thanks,
Yilun
> + };
>
> /* PCIe Requester ID == Linux "devid" */
> ide->rid_start = pci_dev_id(pdev);
