Re: [PATCH v5 03/17] iommu/arm-smmu-v3: Move arm_smmu_rmr_install_bypass_ste()

Mostafa Saleh
2024-02-13T15:38:06Z

Hi Jason,

On Tue, Feb 06, 2024 at 11:12:40AM -0400, Jason Gunthorpe wrote:
> Logically arm_smmu_init_strtab() is the function that allocates and
> populates the stream table with the initial value of the STEs. After this
> function returns the stream table should be fully ready.
>
> arm_smmu_rmr_install_bypass_ste() adjusts the initial stream table to force
> any SIDs that the FW says have IOMMU_RESV_DIRECT to use bypass. This
> ensures there is no disruption to the identity mapping during boot.
>
> Put arm_smmu_rmr_install_bypass_ste() into arm_smmu_init_strtab(), it
> already executes immediately after arm_smmu_init_strtab().
>
> No functional change intended.

I think arm_smmu_init_strtab is quite low level to abstract FW configuration in it.
For example in KVM[1] we'd re-use a big part of this driver and rely on similar
low-level functions. But no strong opinion.

[1] https://lore.kernel.org/kvmarm/20230201125328.2186498-1-jean-philippe@linaro.org/

Thanks,
Mostafa

> Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Tested-by: Moritz Fischer <moritzf@google.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 6123e5ad95822c..2ab36dcf7c61f5 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -101,6 +101,8 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
> { 0, NULL},
> };
>
> +static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu);
> +
> static void parse_driver_options(struct arm_smmu_device *smmu)
> {
> int i = 0;
> @@ -3256,6 +3258,7 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)
> cfg->strtab_base_cfg = reg;
>
> arm_smmu_init_bypass_stes(strtab, cfg->num_l1_ents);
> +
> return 0;
> }
>
> @@ -3279,6 +3282,8 @@ static int arm_smmu_init_strtab(struct arm_smmu_device *smmu)
>
> ida_init(&smmu->vmid_map);
>
> + /* Check for RMRs and install bypass STEs if any */
> + arm_smmu_rmr_install_bypass_ste(smmu);
> return 0;
> }
>
> @@ -4073,9 +4078,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> /* Record our private device structure */
> platform_set_drvdata(pdev, smmu);
>
> - /* Check for RMRs and install bypass STEs if any */
> - arm_smmu_rmr_install_bypass_ste(smmu);
> -
> /* Reset the device */
> ret = arm_smmu_device_reset(smmu, bypass);
> if (ret)
> --
> 2.43.0