* [Query] ACS enablement in the DT based boot flow @ 2024-04-01 10:40 Vidya Sagar 2024-04-10 7:40 ` Vidya Sagar 2024-04-10 19:28 ` Bjorn Helgaas 0 siblings, 2 replies; 4+ messages in thread From: Vidya Sagar @ 2024-04-01 10:40 UTC (permalink / raw) To: Bjorn Helgaas, Rafael J. Wysocki, Krzysztof Wilczyński, Lorenzo Pieralisi Cc: linux-pci, linux-kernel, Manikanta Maddireddy, Shanker Donthineni, Krishna Thota Hi folks, ACS (Access Control Services) is configured for a PCI device through pci_enable_acs(). The first thing pci_enable_acs() checks for is whether the global flag 'pci_acs_enable' is set or not. The global flag 'pci_acs_enable' is set by the function pci_request_acs(). pci_enable_acs() function is called whenever a new PCI device is added to the system pci_enable_acs+0x4c/0x2a4 pci_acs_init+0x38/0x60 pci_device_add+0x1a0/0x670 pci_scan_single_device+0xc4/0x100 pci_scan_slot+0x6c/0x1e0 pci_scan_child_bus_extend+0x48/0x2e0 pci_scan_root_bus_bridge+0x64/0xf0 pci_host_probe+0x18/0xd0 In the case of a system that boots using device-tree blob, pci_request_acs() is called when the device driver binds with the respective device of_iommu_configure+0xf4/0x230 of_dma_configure_id+0x110/0x340 pci_dma_configure+0x54/0x120 really_probe+0x80/0x3e0 __driver_probe_device+0x88/0x1c0 driver_probe_device+0x3c/0x140 __device_attach_driver+0xe8/0x1e0 bus_for_each_drv+0x78/0xf0 __device_attach+0x104/0x1e0 device_attach+0x14/0x30 pci_bus_add_device+0x50/0xd0 pci_bus_add_devices+0x38/0x90 pci_host_probe+0x40/0xd0 Since the device addition always happens first followed by the driver binding, this flow effectively makes sure that ACS never gets enabled. Ideally, I would expect the pci_request_acs() get called (probably by the OF framework itself) before calling pci_enable_acs(). This happens in the ACPI flow where pci_request_acs() is called during IORT node initialization (i.e. iort_init_platform_devices() function). Is this understanding correct? If yes, would it make sense to call pci_request_acs() during OF initialization (similar to IORT initialization in ACPI flow)? Thanks, Vidya Sagar ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Query] ACS enablement in the DT based boot flow 2024-04-01 10:40 [Query] ACS enablement in the DT based boot flow Vidya Sagar @ 2024-04-10 7:40 ` Vidya Sagar 2024-04-10 19:28 ` Bjorn Helgaas 1 sibling, 0 replies; 4+ messages in thread From: Vidya Sagar @ 2024-04-10 7:40 UTC (permalink / raw) To: Bjorn Helgaas, Rafael J. Wysocki, Krzysztof Wilczyński, Lorenzo Pieralisi Cc: linux-pci, linux-kernel, Manikanta Maddireddy, Shanker Donthineni, Krishna Thota Hi folks, any thoughts on this? On 01-04-2024 16:10, Vidya Sagar wrote: > Hi folks, > ACS (Access Control Services) is configured for a PCI device through pci_enable_acs(). > The first thing pci_enable_acs() checks for is whether the global flag 'pci_acs_enable' is set or not. > The global flag 'pci_acs_enable' is set by the function pci_request_acs(). > > pci_enable_acs() function is called whenever a new PCI device is added to the system > > pci_enable_acs+0x4c/0x2a4 > pci_acs_init+0x38/0x60 > pci_device_add+0x1a0/0x670 > pci_scan_single_device+0xc4/0x100 > pci_scan_slot+0x6c/0x1e0 > pci_scan_child_bus_extend+0x48/0x2e0 > pci_scan_root_bus_bridge+0x64/0xf0 > pci_host_probe+0x18/0xd0 > > In the case of a system that boots using device-tree blob, pci_request_acs() is called when the > device driver binds with the respective device > > of_iommu_configure+0xf4/0x230 > of_dma_configure_id+0x110/0x340 > pci_dma_configure+0x54/0x120 > really_probe+0x80/0x3e0 > __driver_probe_device+0x88/0x1c0 > driver_probe_device+0x3c/0x140 > __device_attach_driver+0xe8/0x1e0 > bus_for_each_drv+0x78/0xf0 > __device_attach+0x104/0x1e0 > device_attach+0x14/0x30 > pci_bus_add_device+0x50/0xd0 > pci_bus_add_devices+0x38/0x90 > pci_host_probe+0x40/0xd0 > > Since the device addition always happens first followed by the driver binding, this flow effectively > makes sure that ACS never gets enabled. > > Ideally, I would expect the pci_request_acs() get called (probably by the OF framework itself) before > calling pci_enable_acs(). > > This happens in the ACPI flow where pci_request_acs() is called during IORT node > initialization (i.e. iort_init_platform_devices() function). > > Is this understanding correct? If yes, would it make sense to call pci_request_acs() during > OF initialization (similar to IORT initialization in ACPI flow)? > > Thanks, > Vidya Sagar ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Query] ACS enablement in the DT based boot flow 2024-04-01 10:40 [Query] ACS enablement in the DT based boot flow Vidya Sagar 2024-04-10 7:40 ` Vidya Sagar @ 2024-04-10 19:28 ` Bjorn Helgaas 2024-04-28 7:23 ` Will Deacon 1 sibling, 1 reply; 4+ messages in thread From: Bjorn Helgaas @ 2024-04-10 19:28 UTC (permalink / raw) To: Vidya Sagar Cc: Rafael J. Wysocki, Krzysztof Wilczyński, Lorenzo Pieralisi, linux-pci, linux-kernel, Manikanta Maddireddy, Shanker Donthineni, Krishna Thota, Will Deacon, Joerg Roedel [+cc Will, Joerg] On Mon, Apr 01, 2024 at 10:40:15AM +0000, Vidya Sagar wrote: > Hi folks, > ACS (Access Control Services) is configured for a PCI device through > pci_enable_acs(). The first thing pci_enable_acs() checks for is > whether the global flag 'pci_acs_enable' is set or not. The global > flag 'pci_acs_enable' is set by the function pci_request_acs(). > > pci_enable_acs() function is called whenever a new PCI device is > added to the system > > pci_enable_acs+0x4c/0x2a4 > pci_acs_init+0x38/0x60 > pci_device_add+0x1a0/0x670 > pci_scan_single_device+0xc4/0x100 > pci_scan_slot+0x6c/0x1e0 > pci_scan_child_bus_extend+0x48/0x2e0 > pci_scan_root_bus_bridge+0x64/0xf0 > pci_host_probe+0x18/0xd0 > > In the case of a system that boots using device-tree blob, > pci_request_acs() is called when the device driver binds with the > respective device > > of_iommu_configure+0xf4/0x230 > of_dma_configure_id+0x110/0x340 > pci_dma_configure+0x54/0x120 > really_probe+0x80/0x3e0 > __driver_probe_device+0x88/0x1c0 > driver_probe_device+0x3c/0x140 > __device_attach_driver+0xe8/0x1e0 > bus_for_each_drv+0x78/0xf0 > __device_attach+0x104/0x1e0 > device_attach+0x14/0x30 > pci_bus_add_device+0x50/0xd0 > pci_bus_add_devices+0x38/0x90 > pci_host_probe+0x40/0xd0 > > Since the device addition always happens first followed by the > driver binding, this flow effectively makes sure that ACS never gets > enabled. > > Ideally, I would expect the pci_request_acs() get called (probably > by the OF framework itself) before calling pci_enable_acs(). > > This happens in the ACPI flow where pci_request_acs() is called > during IORT node initialization (i.e. iort_init_platform_devices() > function). > > Is this understanding correct? If yes, would it make sense to call > pci_request_acs() during OF initialization (similar to IORT > initialization in ACPI flow)? Your understanding looks correct to me. My call graph notes, FWIW: mem_init pci_iommu_alloc # x86 only amd_iommu_detect # init_state = IOMMU_START_STATE iommu_go_to_state(IOMMU_IVRS_DETECTED) state_next switch (init_state) case IOMMU_START_STATE: detect_ivrs pci_request_acs pci_acs_enable = 1 # <-- detect_intel_iommu pci_request_acs pci_acs_enable = 1 # <-- pci_scan_single_device # PCI enumeration ... pci_init_capabilities pci_acs_init pci_enable_acs if (pci_acs_enable) # <-- pci_std_enable_acs __driver_probe_device really_probe pci_dma_configure # pci_bus_type.dma_configure if (OF) of_dma_configure of_dma_configure_id of_iommu_configure pci_request_acs # <-- 6bf6c24720d3 iommu_probe_device else if (ACPI) acpi_dma_configure acpi_dma_configure_id acpi_iommu_configure_id iommu_probe_device The pci_request_acs() in of_iommu_configure(), which happens too late to affect pci_enable_acs(), was added by 6bf6c24720d3 ("iommu/of: Request ACS from the PCI core when configuring IOMMU linkage"), so I cc'd Will and Joerg. I don't know if that *used* to work and got broken somehow, or if it never worked as intended. Bjorn ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Query] ACS enablement in the DT based boot flow 2024-04-10 19:28 ` Bjorn Helgaas @ 2024-04-28 7:23 ` Will Deacon 0 siblings, 0 replies; 4+ messages in thread From: Will Deacon @ 2024-04-28 7:23 UTC (permalink / raw) To: Bjorn Helgaas Cc: Vidya Sagar, Rafael J. Wysocki, Krzysztof Wilczyński, Lorenzo Pieralisi, linux-pci, linux-kernel, Manikanta Maddireddy, Shanker Donthineni, Krishna Thota, Joerg Roedel On Wed, Apr 10, 2024 at 02:28:40PM -0500, Bjorn Helgaas wrote: > [+cc Will, Joerg] > > On Mon, Apr 01, 2024 at 10:40:15AM +0000, Vidya Sagar wrote: > > Hi folks, > > ACS (Access Control Services) is configured for a PCI device through > > pci_enable_acs(). The first thing pci_enable_acs() checks for is > > whether the global flag 'pci_acs_enable' is set or not. The global > > flag 'pci_acs_enable' is set by the function pci_request_acs(). > > > > pci_enable_acs() function is called whenever a new PCI device is > > added to the system > > > > pci_enable_acs+0x4c/0x2a4 > > pci_acs_init+0x38/0x60 > > pci_device_add+0x1a0/0x670 > > pci_scan_single_device+0xc4/0x100 > > pci_scan_slot+0x6c/0x1e0 > > pci_scan_child_bus_extend+0x48/0x2e0 > > pci_scan_root_bus_bridge+0x64/0xf0 > > pci_host_probe+0x18/0xd0 > > > > In the case of a system that boots using device-tree blob, > > pci_request_acs() is called when the device driver binds with the > > respective device > > > > of_iommu_configure+0xf4/0x230 > > of_dma_configure_id+0x110/0x340 > > pci_dma_configure+0x54/0x120 > > really_probe+0x80/0x3e0 > > __driver_probe_device+0x88/0x1c0 > > driver_probe_device+0x3c/0x140 > > __device_attach_driver+0xe8/0x1e0 > > bus_for_each_drv+0x78/0xf0 > > __device_attach+0x104/0x1e0 > > device_attach+0x14/0x30 > > pci_bus_add_device+0x50/0xd0 > > pci_bus_add_devices+0x38/0x90 > > pci_host_probe+0x40/0xd0 > > > > Since the device addition always happens first followed by the > > driver binding, this flow effectively makes sure that ACS never gets > > enabled. > > > > Ideally, I would expect the pci_request_acs() get called (probably > > by the OF framework itself) before calling pci_enable_acs(). > > > > This happens in the ACPI flow where pci_request_acs() is called > > during IORT node initialization (i.e. iort_init_platform_devices() > > function). > > > > Is this understanding correct? If yes, would it make sense to call > > pci_request_acs() during OF initialization (similar to IORT > > initialization in ACPI flow)? > > Your understanding looks correct to me. My call graph notes, FWIW: > > mem_init > pci_iommu_alloc # x86 only > amd_iommu_detect # init_state = IOMMU_START_STATE > iommu_go_to_state(IOMMU_IVRS_DETECTED) > state_next > switch (init_state) > case IOMMU_START_STATE: > detect_ivrs > pci_request_acs > pci_acs_enable = 1 # <-- > detect_intel_iommu > pci_request_acs > pci_acs_enable = 1 # <-- > > pci_scan_single_device # PCI enumeration > ... > pci_init_capabilities > pci_acs_init > pci_enable_acs > if (pci_acs_enable) # <-- > pci_std_enable_acs > > __driver_probe_device > really_probe > pci_dma_configure # pci_bus_type.dma_configure > if (OF) > of_dma_configure > of_dma_configure_id > of_iommu_configure > pci_request_acs # <-- 6bf6c24720d3 > iommu_probe_device > else if (ACPI) > acpi_dma_configure > acpi_dma_configure_id > acpi_iommu_configure_id > iommu_probe_device > > The pci_request_acs() in of_iommu_configure(), which happens too late > to affect pci_enable_acs(), was added by 6bf6c24720d3 ("iommu/of: > Request ACS from the PCI core when configuring IOMMU linkage"), so I > cc'd Will and Joerg. I don't know if that *used* to work and got > broken somehow, or if it never worked as intended. I don't have any way to test this, but I'm supportive of having the same flow for DT and ACPI-based flows. Vidya, are you able to cook a patch? Will ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-28 7:23 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-01 10:40 [Query] ACS enablement in the DT based boot flow Vidya Sagar 2024-04-10 7:40 ` Vidya Sagar 2024-04-10 19:28 ` Bjorn Helgaas 2024-04-28 7:23 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).