| Message ID | 173343744264.1074769.10935494914881159519.stgit@dwillia2-xfh.jf.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PCI/TSM: Core infrastructure for PCI device security (TDISP) | expand |
Hi Dan, Dan Williams <dan.j.williams@intel.com> writes: > +int pci_ide_stream_setup(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); > + int mem = 0, rc; > + > + if (ide->stream_id < 0 || ide->stream_id > U8_MAX) { > + pci_err(pdev, "Setup fail: Invalid stream id: %d\n", ide->stream_id); > + return -ENXIO; > + } > + > + if (test_and_set_bit_lock(ide->stream_id, hb->ide_stream_ids)) { > + pci_err(pdev, "Setup fail: Busy stream id: %d\n", > + ide->stream_id); > + return -EBUSY; > + } > + Considering we are using the hostbridge ide_stream_ids bitmap, why is the stream_id allocation not generic? ie, any reason why a stream id alloc like below will not work? static int pcie_ide_sel_streamid_alloc(struct pci_dev *pdev) { int stream_id; struct pci_host_bridge *hb; hb = pci_find_host_bridge(pdev->bus); stream_id = find_first_zero_bit(hb->ide_stream_ids, hb->nr_ide_streams); if (stream_id >= hb->nr_ide_streams) return -EBUSY; return stream_id; } -aneesh
Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes: > Hi Dan, > > Dan Williams <dan.j.williams@intel.com> writes: >> +int pci_ide_stream_setup(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); >> + int mem = 0, rc; >> + >> + if (ide->stream_id < 0 || ide->stream_id > U8_MAX) { >> + pci_err(pdev, "Setup fail: Invalid stream id: %d\n", ide->stream_id); >> + return -ENXIO; >> + } >> + >> + if (test_and_set_bit_lock(ide->stream_id, hb->ide_stream_ids)) { >> + pci_err(pdev, "Setup fail: Busy stream id: %d\n", >> + ide->stream_id); >> + return -EBUSY; >> + } >> + > > Considering we are using the hostbridge ide_stream_ids bitmap, why is > the stream_id allocation not generic? ie, any reason why a stream id alloc > like below will not work? > > static int pcie_ide_sel_streamid_alloc(struct pci_dev *pdev) > { > int stream_id; > struct pci_host_bridge *hb; > > hb = pci_find_host_bridge(pdev->bus); > > stream_id = find_first_zero_bit(hb->ide_stream_ids, hb->nr_ide_streams); > if (stream_id >= hb->nr_ide_streams) > return -EBUSY; > > return stream_id; > } > Also wondering should the stream id be unique at the rootport level? ie for a config like below # pwd /sys/devices/platform/40000000.pci/pci0000:00 # ls 0000:00:01.0 available_secure_streams power 0000:00:02.0 pci_bus uevent # lspci 00:01.0 PCI bridge: ARM Device 0def 00:02.0 PCI bridge: ARM Device 0def 01:00.0 Unassigned class [ff00]: ARM Device ff80 02:00.0 SATA controller: Device 0abc:aced (rev 01) # # lspci -t -[0000:00]-+-01.0-[01]----00.0 \-02.0-[02]----00.0 # I should be able to use the same stream id to program both the rootports? -aneesh
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/Documentation/ABI/testing/sysfs-devices-pci-host-bridge b/Documentation/ABI/testing/sysfs-devices-pci-host-bridge > new file mode 100644 > index 000000000000..15dafb46b176 > --- /dev/null > +++ 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@vger.kernel.org > +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. > + > +What: pciDDDDD:BB/firmware_node > +Date: December, 2024 > +Contact: linux-pci@vger.kernel.org > +Description: > + (RO) Symlink to the platform firmware device object "companion" > + of the host bridge. For example, an ACPI device with an _HID of > + PNP0A08 (/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00). > + > +What: pciDDDDD:BB/streamN:DDDDD:BB:DD:F > +Date: December, 2024 > +Contact: linux-pci@vger.kernel.org > +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. > 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 > @@ -5,6 +5,9 @@ > > #define dev_fmt(fmt) "PCI/IDE: " fmt > #include <linux/pci.h> > +#include <linux/sysfs.h> > +#include <linux/pci-ide.h> > +#include <linux/bitfield.h> > #include "pci.h" > > static int sel_ide_offset(u16 cap, int stream_id, int nr_ide_mem) > @@ -71,3 +74,192 @@ void pci_ide_init(struct pci_dev *pdev) > pdev->sel_ide_cap = sel_ide_cap; > pdev->nr_ide_mem = nr_ide_mem; > } > + > +void pci_init_host_bridge_ide(struct pci_host_bridge *hb) > +{ > + hb->ide_stream_res = > + DEFINE_RES_MEM_NAMED(0, 0, "IDE Address Association"); out of curiosity - should it be DEFINE_RES_MEM_NAMED(0, UINT_MAX, "IDE Address Association");? > +} > + > +/* > + * Retrieve stream association parameters for devid (RID) and resources > + * (device address ranges) > + */ > +void pci_ide_stream_probe(struct pci_dev *pdev, struct pci_ide *ide) > +{ > + int num_vf = pci_num_vf(pdev); > + > + *ide = (struct pci_ide) { .stream_id = -1 }; > + > + if (pdev->fm_enabled) > + ide->domain = pci_domain_nr(pdev->bus); > + ide->devid_start = pci_dev_id(pdev); > + > + /* for SR-IOV case, cover all VFs */ > + if (num_vf) > + ide->devid_end = PCI_DEVID(pci_iov_virtfn_bus(pdev, num_vf), > + pci_iov_virtfn_devfn(pdev, num_vf)); > + else > + ide->devid_end = ide->devid_start; > + > + /* TODO: address association probing... */ > +} > +EXPORT_SYMBOL_GPL(pci_ide_stream_probe); > + > +static void __pci_ide_stream_teardown(struct pci_dev *pdev, struct pci_ide *ide) > +{ > + int pos; > + > + pos = sel_ide_offset(pdev->sel_ide_cap, ide->stream_id, > + pdev->nr_ide_mem); > + > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_CTL, 0); > + for (int i = ide->nr_mem - 1; i >= 0; i--) { > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_3(i), 0); > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_2(i), 0); > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_1(i), 0); > + } > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_2, 0); > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_1, 0); > +} > + > +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++) { > + val = FIELD_PREP(PCI_IDE_SEL_ADDR_1_VALID, 1) | > + FIELD_PREP(PCI_IDE_SEL_ADDR_1_BASE_LOW_MASK, > + lower_32_bits(ide->mem[i].start) >> > + PCI_IDE_SEL_ADDR_1_BASE_LOW_SHIFT) | > + FIELD_PREP(PCI_IDE_SEL_ADDR_1_LIMIT_LOW_MASK, > + lower_32_bits(ide->mem[i].end) >> > + PCI_IDE_SEL_ADDR_1_LIMIT_LOW_SHIFT); > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_1(i), val); > + > + val = upper_32_bits(ide->mem[i].end); > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_2(i), val); > + > + val = upper_32_bits(ide->mem[i].start); > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_3(i), val); > + } > +} > + > +/* > + * Establish IDE stream parameters in @pdev and, optionally, its root port > + */ > +int pci_ide_stream_setup(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); > + int mem = 0, rc; > + > + if (ide->stream_id < 0 || ide->stream_id > U8_MAX) { > + pci_err(pdev, "Setup fail: Invalid stream id: %d\n", ide->stream_id); > + return -ENXIO; > + } > + > + if (test_and_set_bit_lock(ide->stream_id, hb->ide_stream_ids)) { > + pci_err(pdev, "Setup fail: Busy stream id: %d\n", > + ide->stream_id); > + return -EBUSY; > + } > + > + ide->name = kasprintf(GFP_KERNEL, "stream%d:%s", ide->stream_id, > + dev_name(&pdev->dev)); > + if (!ide->name) { > + rc = -ENOMEM; > + goto err_name; > + } > + > + rc = sysfs_create_link(&hb->dev.kobj, &pdev->dev.kobj, ide->name); > + if (rc) > + goto err_link; > + > + for (mem = 0; mem < ide->nr_mem; mem++) > + if (!__request_region(&hb->ide_stream_res, ide->mem[mem].start, > + range_len(&ide->mem[mem]), ide->name, > + 0)) { > + pci_err(pdev, > + "Setup fail: stream%d: address association conflict [%#llx-%#llx]\n", > + ide->stream_id, ide->mem[mem].start, > + ide->mem[mem].end); > + > + rc = -EBUSY; > + goto err; > + } > + > + __pci_ide_stream_setup(pdev, ide); > + if (flags & PCI_IDE_SETUP_ROOT_PORT) > + __pci_ide_stream_setup(rp, ide); > + > + return 0; > +err: > + for (; mem >= 0; mem--) > + __release_region(&hb->ide_stream_res, ide->mem[mem].start, > + range_len(&ide->mem[mem])); > + sysfs_remove_link(&hb->dev.kobj, ide->name); > +err_link: > + kfree(ide->name); > +err_name: > + clear_bit_unlock(ide->stream_id, hb->ide_stream_ids); > + return rc; > +} > +EXPORT_SYMBOL_GPL(pci_ide_stream_setup); > + > +void pci_ide_enable_stream(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_CTL_ID_MASK, ide->stream_id) | > + FIELD_PREP(PCI_IDE_SEL_CTL_DEFAULT, 1); FIELD_PREP(PCI_IDE_SEL_CTL_EN) is missing here. And no PCI_IDE_SETUP_ROOT_PORT here, is the platform expected to enable it explicitely? If yes, then we do not need PCI_IDE_SETUP_ROOT_PORT really. If no, then this needs to enable the stream on the rootport. Also, my test device wants PCI_IDE_SEL_CTL_TEE_LIMITED to work, I wonder how to showel it in here, add a "unsigned dev_sel_ctl" to pci_ide? And the same comment for PCI_IDE_SEL_CTL_CFG_EN on the rootport. "unsigned rootport_sel_ctl"? > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_CTL, val); > +} > +EXPORT_SYMBOL_GPL(pci_ide_enable_stream); > + > +void pci_ide_disable_stream(struct pci_dev *pdev, struct pci_ide *ide) > +{ > + int pos; > + > + pos = sel_ide_offset(pdev->sel_ide_cap, ide->stream_id, > + pdev->nr_ide_mem); > + > + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_CTL, 0); > +} > +EXPORT_SYMBOL_GPL(pci_ide_disable_stream); > + > +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); > + > + for (int i = ide->nr_mem - 1; i >= 0; i--) > + __release_region(&hb->ide_stream_res, ide->mem[i].start, > + range_len(&ide->mem[i])); > + sysfs_remove_link(&hb->dev.kobj, ide->name); > + kfree(ide->name); > + clear_bit_unlock(ide->stream_id, hb->ide_stream_ids); > +} > +EXPORT_SYMBOL_GPL(pci_ide_stream_teardown); > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index 6565eb72ded2..b267fabfd542 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -465,8 +465,12 @@ static inline void pci_npem_remove(struct pci_dev *dev) { } > > #ifdef CONFIG_PCI_IDE > void pci_ide_init(struct pci_dev *dev); > +void pci_init_host_bridge_ide(struct pci_host_bridge *bridge); > #else > static inline void pci_ide_init(struct pci_dev *dev) { } > +static inline void pci_init_host_bridge_ide(struct pci_host_bridge *bridge) > +{ > +} > #endif > > #ifdef CONFIG_PCI_TSM > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 6c1fe6354d26..667faa18ced2 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -608,6 +608,7 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) > bridge->native_dpc = 1; > bridge->domain_nr = PCI_DOMAIN_NR_NOT_SET; > bridge->native_cxl_error = 1; > + pci_init_host_bridge_ide(bridge); > > device_initialize(&bridge->dev); > } > diff --git a/include/linux/pci-ide.h b/include/linux/pci-ide.h > new file mode 100644 > index 000000000000..24e08a413645 > --- /dev/null > +++ b/include/linux/pci-ide.h > @@ -0,0 +1,33 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */ > + > +/* PCIe 6.2 section 6.33 Integrity & Data Encryption (IDE) */ > + > +#ifndef __PCI_IDE_H__ > +#define __PCI_IDE_H__ > + > +#include <linux/range.h> > + > +struct pci_ide { > + int domain; Not much point in caching this, real easy to calculate in that one spot. Thanks, ps. I'll probably be commenting more on the same things as I try using all this :) > + u16 devid_start; > + u16 devid_end; > + int stream_id; > + const char *name; > + int nr_mem; > + struct range mem[16]; > +}; > + > +void pci_ide_stream_probe(struct pci_dev *pdev, struct pci_ide *ide); > + > +enum pci_ide_flags { > + PCI_IDE_SETUP_ROOT_PORT = BIT(0), > +}; > + > +int pci_ide_stream_setup(struct pci_dev *pdev, struct pci_ide *ide, > + enum pci_ide_flags flags); > +void pci_ide_stream_teardown(struct pci_dev *pdev, struct pci_ide *ide, > + enum pci_ide_flags flags); > +void pci_ide_enable_stream(struct pci_dev *pdev, struct pci_ide *ide); > +void pci_ide_disable_stream(struct pci_dev *pdev, struct pci_ide *ide); > +#endif /* __PCI_IDE_H__ */ > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 10d035395a43..5d9fc498bc70 100644 > --- a/include/linux/pci.h > +++ 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 */ > +#endif > u8 (*swizzle_irq)(struct pci_dev *, u8 *); /* Platform IRQ swizzler */ > int (*map_irq)(const struct pci_dev *, u8, u8); > void (*release_fn)(struct pci_host_bridge *); >
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@vger.kernel.org > +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@vger.kernel.org > +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.
> +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++) { > + val = FIELD_PREP(PCI_IDE_SEL_ADDR_1_VALID, 1) | > + FIELD_PREP(PCI_IDE_SEL_ADDR_1_BASE_LOW_MASK, > + lower_32_bits(ide->mem[i].start) >> > + PCI_IDE_SEL_ADDR_1_BASE_LOW_SHIFT) | > + FIELD_PREP(PCI_IDE_SEL_ADDR_1_LIMIT_LOW_MASK, > + lower_32_bits(ide->mem[i].end) >> > + PCI_IDE_SEL_ADDR_1_LIMIT_LOW_SHIFT); Oh, I missunderstood the _LOW_SHIFT Macros. But still think if they could be moved out of pci_reg.h. Placing in pci_reg.h makes me think they are some register field offsets. Thanks, Yilun
diff --git a/Documentation/ABI/testing/sysfs-devices-pci-host-bridge b/Documentation/ABI/testing/sysfs-devices-pci-host-bridge new file mode 100644 index 000000000000..15dafb46b176 --- /dev/null +++ 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@vger.kernel.org +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. + +What: pciDDDDD:BB/firmware_node +Date: December, 2024 +Contact: linux-pci@vger.kernel.org +Description: + (RO) Symlink to the platform firmware device object "companion" + of the host bridge. For example, an ACPI device with an _HID of + PNP0A08 (/sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00). + +What: pciDDDDD:BB/streamN:DDDDD:BB:DD:F +Date: December, 2024 +Contact: linux-pci@vger.kernel.org +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. 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 @@ -5,6 +5,9 @@ #define dev_fmt(fmt) "PCI/IDE: " fmt #include <linux/pci.h> +#include <linux/sysfs.h> +#include <linux/pci-ide.h> +#include <linux/bitfield.h> #include "pci.h" static int sel_ide_offset(u16 cap, int stream_id, int nr_ide_mem) @@ -71,3 +74,192 @@ void pci_ide_init(struct pci_dev *pdev) pdev->sel_ide_cap = sel_ide_cap; pdev->nr_ide_mem = nr_ide_mem; } + +void pci_init_host_bridge_ide(struct pci_host_bridge *hb) +{ + hb->ide_stream_res = + DEFINE_RES_MEM_NAMED(0, 0, "IDE Address Association"); +} + +/* + * Retrieve stream association parameters for devid (RID) and resources + * (device address ranges) + */ +void pci_ide_stream_probe(struct pci_dev *pdev, struct pci_ide *ide) +{ + int num_vf = pci_num_vf(pdev); + + *ide = (struct pci_ide) { .stream_id = -1 }; + + if (pdev->fm_enabled) + ide->domain = pci_domain_nr(pdev->bus); + ide->devid_start = pci_dev_id(pdev); + + /* for SR-IOV case, cover all VFs */ + if (num_vf) + ide->devid_end = PCI_DEVID(pci_iov_virtfn_bus(pdev, num_vf), + pci_iov_virtfn_devfn(pdev, num_vf)); + else + ide->devid_end = ide->devid_start; + + /* TODO: address association probing... */ +} +EXPORT_SYMBOL_GPL(pci_ide_stream_probe); + +static void __pci_ide_stream_teardown(struct pci_dev *pdev, struct pci_ide *ide) +{ + int pos; + + pos = sel_ide_offset(pdev->sel_ide_cap, ide->stream_id, + pdev->nr_ide_mem); + + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_CTL, 0); + for (int i = ide->nr_mem - 1; i >= 0; i--) { + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_3(i), 0); + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_2(i), 0); + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_1(i), 0); + } + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_2, 0); + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_1, 0); +} + +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++) { + val = FIELD_PREP(PCI_IDE_SEL_ADDR_1_VALID, 1) | + FIELD_PREP(PCI_IDE_SEL_ADDR_1_BASE_LOW_MASK, + lower_32_bits(ide->mem[i].start) >> + PCI_IDE_SEL_ADDR_1_BASE_LOW_SHIFT) | + FIELD_PREP(PCI_IDE_SEL_ADDR_1_LIMIT_LOW_MASK, + lower_32_bits(ide->mem[i].end) >> + PCI_IDE_SEL_ADDR_1_LIMIT_LOW_SHIFT); + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_1(i), val); + + val = upper_32_bits(ide->mem[i].end); + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_2(i), val); + + val = upper_32_bits(ide->mem[i].start); + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_ADDR_3(i), val); + } +} + +/* + * Establish IDE stream parameters in @pdev and, optionally, its root port + */ +int pci_ide_stream_setup(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); + int mem = 0, rc; + + if (ide->stream_id < 0 || ide->stream_id > U8_MAX) { + pci_err(pdev, "Setup fail: Invalid stream id: %d\n", ide->stream_id); + return -ENXIO; + } + + if (test_and_set_bit_lock(ide->stream_id, hb->ide_stream_ids)) { + pci_err(pdev, "Setup fail: Busy stream id: %d\n", + ide->stream_id); + return -EBUSY; + } + + ide->name = kasprintf(GFP_KERNEL, "stream%d:%s", ide->stream_id, + dev_name(&pdev->dev)); + if (!ide->name) { + rc = -ENOMEM; + goto err_name; + } + + rc = sysfs_create_link(&hb->dev.kobj, &pdev->dev.kobj, ide->name); + if (rc) + goto err_link; + + for (mem = 0; mem < ide->nr_mem; mem++) + if (!__request_region(&hb->ide_stream_res, ide->mem[mem].start, + range_len(&ide->mem[mem]), ide->name, + 0)) { + pci_err(pdev, + "Setup fail: stream%d: address association conflict [%#llx-%#llx]\n", + ide->stream_id, ide->mem[mem].start, + ide->mem[mem].end); + + rc = -EBUSY; + goto err; + } + + __pci_ide_stream_setup(pdev, ide); + if (flags & PCI_IDE_SETUP_ROOT_PORT) + __pci_ide_stream_setup(rp, ide); + + return 0; +err: + for (; mem >= 0; mem--) + __release_region(&hb->ide_stream_res, ide->mem[mem].start, + range_len(&ide->mem[mem])); + sysfs_remove_link(&hb->dev.kobj, ide->name); +err_link: + kfree(ide->name); +err_name: + clear_bit_unlock(ide->stream_id, hb->ide_stream_ids); + return rc; +} +EXPORT_SYMBOL_GPL(pci_ide_stream_setup); + +void pci_ide_enable_stream(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_CTL_ID_MASK, ide->stream_id) | + FIELD_PREP(PCI_IDE_SEL_CTL_DEFAULT, 1); + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_CTL, val); +} +EXPORT_SYMBOL_GPL(pci_ide_enable_stream); + +void pci_ide_disable_stream(struct pci_dev *pdev, struct pci_ide *ide) +{ + int pos; + + pos = sel_ide_offset(pdev->sel_ide_cap, ide->stream_id, + pdev->nr_ide_mem); + + pci_write_config_dword(pdev, pos + PCI_IDE_SEL_CTL, 0); +} +EXPORT_SYMBOL_GPL(pci_ide_disable_stream); + +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); + + for (int i = ide->nr_mem - 1; i >= 0; i--) + __release_region(&hb->ide_stream_res, ide->mem[i].start, + range_len(&ide->mem[i])); + sysfs_remove_link(&hb->dev.kobj, ide->name); + kfree(ide->name); + clear_bit_unlock(ide->stream_id, hb->ide_stream_ids); +} +EXPORT_SYMBOL_GPL(pci_ide_stream_teardown); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 6565eb72ded2..b267fabfd542 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -465,8 +465,12 @@ static inline void pci_npem_remove(struct pci_dev *dev) { } #ifdef CONFIG_PCI_IDE void pci_ide_init(struct pci_dev *dev); +void pci_init_host_bridge_ide(struct pci_host_bridge *bridge); #else static inline void pci_ide_init(struct pci_dev *dev) { } +static inline void pci_init_host_bridge_ide(struct pci_host_bridge *bridge) +{ +} #endif #ifdef CONFIG_PCI_TSM diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 6c1fe6354d26..667faa18ced2 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -608,6 +608,7 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) bridge->native_dpc = 1; bridge->domain_nr = PCI_DOMAIN_NR_NOT_SET; bridge->native_cxl_error = 1; + pci_init_host_bridge_ide(bridge); device_initialize(&bridge->dev); } diff --git a/include/linux/pci-ide.h b/include/linux/pci-ide.h new file mode 100644 index 000000000000..24e08a413645 --- /dev/null +++ b/include/linux/pci-ide.h @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */ + +/* PCIe 6.2 section 6.33 Integrity & Data Encryption (IDE) */ + +#ifndef __PCI_IDE_H__ +#define __PCI_IDE_H__ + +#include <linux/range.h> + +struct pci_ide { + int domain; + u16 devid_start; + u16 devid_end; + int stream_id; + const char *name; + int nr_mem; + struct range mem[16]; +}; + +void pci_ide_stream_probe(struct pci_dev *pdev, struct pci_ide *ide); + +enum pci_ide_flags { + PCI_IDE_SETUP_ROOT_PORT = BIT(0), +}; + +int pci_ide_stream_setup(struct pci_dev *pdev, struct pci_ide *ide, + enum pci_ide_flags flags); +void pci_ide_stream_teardown(struct pci_dev *pdev, struct pci_ide *ide, + enum pci_ide_flags flags); +void pci_ide_enable_stream(struct pci_dev *pdev, struct pci_ide *ide); +void pci_ide_disable_stream(struct pci_dev *pdev, struct pci_ide *ide); +#endif /* __PCI_IDE_H__ */ diff --git a/include/linux/pci.h b/include/linux/pci.h index 10d035395a43..5d9fc498bc70 100644 --- a/include/linux/pci.h +++ 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 */ +#endif u8 (*swizzle_irq)(struct pci_dev *, u8 *); /* Platform IRQ swizzler */ int (*map_irq)(const struct pci_dev *, u8, u8); void (*release_fn)(struct pci_host_bridge *);