On 2017/9/22 17:45, Lorenzo Pieralisi wrote: > On Thu, Sep 21, 2017 at 07:32:58PM +0800, Zhou Wang wrote: >> On 2017/9/20 1:05, Lorenzo Pieralisi wrote: >>> commit f6810c15cf97 ("iommu/arm-smmu: Clean up early-probing >>> workarounds") removed kernel code that was allowing to initialize >>> and probe the SMMU devices early (ie earlier than PCI devices through >>> linker script callback entries) in the boot process because it was not >>> needed any longer in that the SMMU devices/drivers now support deferred >>> probing. >>> >>> Since the SMMUs probe routines are also in charge of requesting global >>> PCI ACS kernel enablement, commit f6810c15cf97 ("iommu/arm-smmu: Clean >>> up early-probing workarounds") also postponed PCI ACS enablement to >>> SMMUs devices probe time, which may be too late given that PCI devices >>> needs to detect if PCI ACS is enabled to init the respective capability >>> through the following call path: >>> >>> pci_device_add() >>> -> pci_init_capabilities() >>> -> pci_enable_acs() >>> >>> Add code in the ACPI IORT SMMU platform devices initialization path >>> (that is called before ACPI PCI enumeration) to detect if an SMMU is >>> present in HW and enable PCI ACS if it actually is, restoring the >>> correct PCI ACS enablement sequencing. >>> >>> Fixes: f6810c15cf97 ("iommu/arm-smmu: Clean up early-probing >>> Signed-workarounds") >>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> >>> Cc: Will Deacon <will.deacon@xxxxxxx> >>> Cc: Robin Murphy <robin.murphy@xxxxxxx> >>> Cc: Zhou Wang <wangzhou1@xxxxxxxxxxxxx> >>> Cc: Alex Williamson <alex.williamson@xxxxxxxxxx> >>> --- >>> drivers/acpi/arm64/iort.c | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >>> index 9565d57..71a7694 100644 >>> --- a/drivers/acpi/arm64/iort.c >>> +++ b/drivers/acpi/arm64/iort.c >>> @@ -1184,6 +1184,7 @@ static void __init iort_init_platform_devices(void) >>> struct acpi_table_iort *iort; >>> struct fwnode_handle *fwnode; >>> int i, ret; >>> + bool smmu_detected = false; >>> >>> /* >>> * iort_table and iort both point to the start of IORT table, but >>> @@ -1218,11 +1219,21 @@ static void __init iort_init_platform_devices(void) >>> acpi_free_fwnode_static(fwnode); >>> return; >>> } >>> + >>> + smmu_detected = true; >>> } >>> >>> iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node, >>> iort_node->length); >>> } >>> + >>> + /* >>> + * If IORT reports an SMMU component make sure PCI ACS is >>> + * requested so that PCI devices can enable it in their >>> + * capabilities. >>> + */ >>> + if (smmu_detected) >>> + pci_request_acs(); >>> } >>> >>> void __init acpi_iort_init(void) >>> >> >> Hi Lorenzo, >> >> I tested this patch, it works well in my HiSilicon Hip08 based system. >> However, setting ACS flag at the stage of SMMU device init seems not good, >> I mean what if in one system there are only platform devices connected to >> SMMU device. > > That's a fair point if you explain to me how current pci_request_acs() > usage copes with your remark above. The current pci_request_acs usage for ARM SMMU-V3 sets ACS flags under CONFIG_PCI. However, as mentioned in your commit message, this setting is too late. For the usage of X86 and AMD, as I am not familiar with the devices used in these two platforms, maybe the default devices in these platforms are PCIe based :) Best, Zhou > > Lorenzo > > . > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html