Re: [PATCH] ACPI/IORT: Fix PCI ACS enablement

[Zhou Wang <wangzhou1@hisilicon.com>]

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@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Zhou Wang <wangzhou1@hisilicon.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> ---
>  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.

Best,
Zhou