Jonathan Cameron wrote: > On Thu, 05 Dec 2024 14:23:51 -0800 > Dan Williams <dan.j.williams@xxxxxxxxx> wrote: > > > Establish just enough emulated PCI infrastructure to register a sample > > TSM (platform security manager) driver and have it discover an IDE + TEE > > (link encryption + device-interface security protocol (TDISP)) capable > > device. > > > > Use the existing a CONFIG_PCI_BRIDGE_EMUL to emulate an IDE capable root > > port, and open code the emulation of an endpoint device via simulated > > configuration cycle responses. > > > > The devsec_tsm driver responds to the PCI core TSM operations as if it > > successfully exercised the given interface security protocol message. > > > > The devsec_bus and devsec_tsm drivers can be loaded in either order to > > reflect cases like SEV-TIO where the TSM is PCI-device firmware, and > > cases like TDX Connect where the TSM is a software agent running on the > > host CPU. > > > > Follow-on patches add common code for TSM managed IDE establishment. For > > now, just successfully complete setup and teardown of the DSM (device > > security manager) context as a building block for management of TDI > > (trusted device interface) instances. > > > > # modprobe devsec_bus > > devsec_bus devsec_bus: PCI host bridge to bus 10000:00 > > pci_bus 10000:00: root bus resource [bus 00-01] > > pci_bus 10000:00: root bus resource [mem 0xf000000000-0xffffffffff 64bit] > > pci 10000:00:00.0: [8086:7075] type 01 class 0x060400 PCIe Root Port > > pci 10000:00:00.0: PCI bridge to [bus 00] > > pci 10000:00:00.0: bridge window [io 0x0000-0x0fff] > > pci 10000:00:00.0: bridge window [mem 0x00000000-0x000fffff] > > pci 10000:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref] > > pci 10000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring > > pci 10000:01:00.0: [8086:ffff] type 00 class 0x000000 PCIe Endpoint > > pci 10000:01:00.0: BAR 0 [mem 0xf000000000-0xf0001fffff 64bit pref] > > pci_doe_abort: pci 10000:01:00.0: DOE: [100] Issuing Abort > > pci_doe_cache_protocols: pci 10000:01:00.0: DOE: [100] Found protocol 0 vid: 1 prot: 1 > > pci 10000:01:00.0: disabling ASPM on pre-1.1 PCIe device. You can enable it with 'pcie_aspm=force' > > pci 10000:00:00.0: PCI bridge to [bus 01] > > pci_bus 10000:01: busn_res: [bus 01] end is updated to 01 > > # modprobe devsec_tsm > > devsec_tsm_pci_probe: pci 10000:01:00.0: devsec: tsm enabled > > __pci_tsm_init: pci 10000:01:00.0: TSM: Device security capabilities detected ( ide tee ), TSM attach > > > > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > Cc: Lukas Wunner <lukas@xxxxxxxxx> > > Cc: Samuel Ortiz <sameo@xxxxxxxxxxxx> > > Cc: Alexey Kardashevskiy <aik@xxxxxxx> > > Cc: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx> > > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> > Hi Dan, > > A few minor comments as I was reading this. Mostly just trying > to get my head around it hence they are all fairly superficial things. > > Jonathan > > > diff --git a/samples/devsec/bus.c b/samples/devsec/bus.c > > new file mode 100644 > > index 000000000000..47dbe4e1b648 > > --- /dev/null > > +++ b/samples/devsec/bus.c > > > > +static void destroy_iomem_pool(void *data) > > There is a devm_gen_pool_create you can probably use. Indeed there is, thanks. [..] > Similar to the case below. I'd rather see a per dev devm_ cleanup > than relying on unified cleanup and that array having null entrees. > Should end up easier to follow. Might require devsec dev to have > a reference back to the pool though. Done for ports and devs. The arrays are used during PCI bus operations. This made me realize that I should be putting the device and port allocation *before* the PCI bus creation to make sure those arrays are dead and idle before the they are invalidated by the port and dev devres actions. [..] > > +static int init_port(struct devsec_port *devsec_port) > > +{ > > + struct pci_bridge_emul *bridge = &devsec_port->bridge; > > + int rc; > > + > > + bridge->conf.vendor = cpu_to_le16(0x8086); > > + bridge->conf.device = cpu_to_le16(0x7075); > > + bridge->subsystem_vendor_id = cpu_to_le16(0x8086); > > + bridge->conf.class_revision = cpu_to_le32(0x1); > > + > > + bridge->conf.pref_mem_base = cpu_to_le16(PCI_PREF_RANGE_TYPE_64); > > + bridge->conf.pref_mem_limit = cpu_to_le16(PCI_PREF_RANGE_TYPE_64); > > + > > + bridge->has_pcie = true; > > + bridge->pcie_conf.devcap = cpu_to_le16(PCI_EXP_DEVCAP_FLR); > > + bridge->pcie_conf.lnksta = cpu_to_le16(PCI_EXP_LNKSTA_CLS_2_5GB); > > + > > + bridge->data = devsec_port; > > + bridge->ops = &devsec_bridge_ops; > Maybe > *bridge = (struct pci_bridge_emul) { > }; > appropriate here. Sure. > > + > > + init_ide(&devsec_port->ide); > > + > > + rc = pci_bridge_emul_init(bridge, 0); > > return pci_bridge_emul_init() unless a later patch is going to add more here. Ok.