Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] iommu/arm-smmu-qcom: unconditional CB 1 reservation for LPAC shifts GMU CB --
commit 51e5e8227679664f11df23a51921048a054a1310 Author: Anna Maniscalco <[email protected]> iommu: arm-smmu-qcom: Fixed mapping between sid and cb for gpu and lpac This commit splits context bank allocation and initialization in arm-smmu-qcom so that the gpu device (stream id 0) is assigned context bank 0 and the lpac device (stream id 1) is assigned context bank 1, while other devices start allocation from context bank 2. > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > index ae6152bddf8fe..9413b748e79d1 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c [ ... ] > @@ -313,12 +330,14 @@ static int qcom_adreno_smmu_alloc_context_bank(struct > arm_smmu_domain *smmu_doma > */ > if (qcom_adreno_smmu_is_gpu_device(dev)) { > start = 0; > - count = 2; > + count = start + 1; > + } else if (qcom_adreno_smmu_is_lpac_device(dev)) { > + start = 1; > + count = start + 1; > } else { > start = 2; > count = smmu->num_context_banks; > } > - > return __arm_smmu_alloc_bitmap(smmu->context_map, start, count); > } [Severity: High] Does unconditionally reserving context bank 1 for a separate lpac device and forcing all other devices into the else branch with start = 2 cause a regression on platforms without a separate qcom,lpac device node? In drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c, qcom_adreno_smmu_alloc_context_bank() now assigns only context bank 0 when qcom_adreno_smmu_is_gpu_device(dev) is true, reserves context bank 1 for a separate device matching qcom_adreno_smmu_is_lpac_device(dev), and starts all other allocations at context bank 2. On platforms that do not have stream id 1 (such as sdm845, sc7180, sm8150, and sm8250), or on platforms where both stream id 0 and stream id 1 are listed on the main gpu node rather than a separate qcom,lpac child node (such as sc8280xp, sm8350, sc7280, sm8450, sm8550, x1e80100, or sm8650 with an older device tree), the main gpu device matches qcom_adreno_smmu_is_gpu_device(dev) first and allocates context bank 0, leaving context bank 1 unallocated. When a6xx_gmu_memory_probe() -> msm_iommu_new() -> iommu_attach_device() -> arm_smmu_init_domain_context() -> qcom_adreno_smmu_alloc_context_bank() subsequently allocates a context bank for the qcom,adreno-gmu device (stream id 5), it falls into the else branch with start = 2 and receives cbndx = 2 instead of cbndx = 1. As noted in commit afc95681c306 ("iommu/arm-smmu-qcom: Add missing GMU entry to match table"), having the gmu on cbndx = 2 instead of cbndx = 1 on sc8280xp causes the smmu to wedge on gpu translation faults. In addition, on platforms with only two context banks available to Linux, such as qcom,sdm630-smmu-v2: drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c:qcom_adreno_smmuv2_cfg_probe() { /* TZ protects several last context banks, hide them from Linux */ if (of_device_is_compatible(smmu->dev->of_node, "qcom,sdm630-smmu-v2") && smmu->num_context_banks == 5) smmu->num_context_banks = 2; } entering the else branch passes start = 2 and count = 2 to __arm_smmu_alloc_bitmap(), which immediately fails even though context bank 1 is free: drivers/iommu/arm/arm-smmu/arm-smmu.h:__arm_smmu_alloc_bitmap() { do { idx = find_next_zero_bit(map, end, start); if (idx == end) return -ENOSPC; } while (test_and_set_bit(idx, map)); } Could context bank 1 only be reserved when a separate lpac device is actually present, or should the else branch still allow allocating from context bank 1 when lpac is not used? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20261029-descriptive-name-lpac-upstream-v2-0-a21a5c850...@gmail.com?part=7
