Re: [PATCH RFC 07/13] iommu: arm-smmu-qcom: Fixed mapping between sid and cb for gpu and lpac

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The context bank allocation logic assigns Context Bank 1 instead of Context Bank 0 to legacy unified GPU devices that declare both SID 0 and SID 1, breaking GPU hardware pagetable switching.
--

commit ae389e90eddaec7861c0838f081fce60156d8874
Author: Anna Maniscalco <anna.maniscalco2000@gmail.com>

iommu: arm-smmu-qcom: Fixed mapping between sid and cb for gpu and lpac

This patch updates the context bank allocation logic for Adreno SMMU devices.
It introduces device identification functions and modifies the context bank
allocator to map context bank 0 to SID 0 and context bank 1 to SID 1.

> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index ae6152bddf8fe..e605a535f328f 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -225,6 +225,25 @@ static void qcom_adreno_smmu_set_prr_addr(const void *cookie, phys_addr_t page_a
>  #define QCOM_ADRENO_SMMU_GPU_SID 0
>  #define QCOM_ADRENO_SMMU_LPAC_SID 1
>  
> +static bool qcom_adreno_smmu_is_lpac_device(struct device *dev)
> +{
> +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +	int i;
> +
> +	/*
> +	 * The GPU will always use SID 0 so that is a handy way to uniquely
> +	 * identify it and configure it for per-instance pagetables
> +	 */
> +	for (i = 0; i < fwspec->num_ids; i++) {
> +		u16 sid = FIELD_GET(ARM_SMMU_SMR_ID, fwspec->ids[i]);
> +
> +		if (sid == QCOM_ADRENO_SMMU_LPAC_SID)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +

[ ... ]

> @@ -312,13 +331,12 @@ static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_doma
>  	 * switch pagetables
>  	 */
>  	if (qcom_adreno_smmu_is_gpu_device(dev)) {
> -		start = 0;
> -		count = 2;
> +		start = !!qcom_adreno_smmu_is_lpac_device(dev);
> +		count = start + 1;

[Severity: High]
Does this break context bank allocation for legacy unified GPU devices?

When probing older supported SoCs where a single unified primary GPU device
node declares both SID 0 and SID 1, qcom_adreno_smmu_is_lpac_device() will
return true because one of the SIDs matches QCOM_ADRENO_SMMU_LPAC_SID.

This calculates start = 1 and count = 2. The __arm_smmu_alloc_bitmap() call
will then search exclusively in the [1, 2) range and allocate Context Bank 1
for the primary GPU domain, completely skipping Context Bank 0.

Since the Adreno GPU hardware natively expects and hardcodes Context Bank 0
for its main pagetable switching via its internal APERTURE, allocating
Context Bank 1 instead of 0 violates hardware constraints. Will this result
in SMMU faults and a frozen GPU on those legacy platforms?

>  	} else {
>  		start = 2;
>  		count = smmu->num_context_banks;
>  	}
> -
>  	return __arm_smmu_alloc_bitmap(smmu->context_map, start, count);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705-descriptive-name-lpac-upstream-v1-0-01d50c3e0c99@gmail.com?part=7