| Message ID | 416c7369fcdce4ebb2a8f12daae234507be27e38.1733406275.git.robin.murphy@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features | expand |
On Thu, Dec 05, 2024 at 01:48:10PM +0000, Robin Murphy wrote: > Since system_supports_gcs() ends up referring to cpucap_is_possible(), > teach the latter about GCS for consistency with similar features. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> Acked-by: Mark Rutland <mark.rutland@arm.com> Looking around, we should probably do the same for HAFT. Mark. > --- > arch/arm64/include/asm/cpucaps.h | 2 ++ > arch/arm64/include/asm/cpufeature.h | 3 +-- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h > index 201a46efd918..cbbf70e0f204 100644 > --- a/arch/arm64/include/asm/cpucaps.h > +++ b/arch/arm64/include/asm/cpucaps.h > @@ -44,6 +44,8 @@ cpucap_is_possible(const unsigned int cap) > return IS_ENABLED(CONFIG_ARM64_TLB_RANGE); > case ARM64_HAS_S1POE: > return IS_ENABLED(CONFIG_ARM64_POE); > + case ARM64_HAS_GCS: > + return IS_ENABLED(CONFIG_ARM64_GCS); > case ARM64_UNMAP_KERNEL_AT_EL0: > return IS_ENABLED(CONFIG_UNMAP_KERNEL_AT_EL0); > case ARM64_WORKAROUND_843419: > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > index b64e49bd9d10..8b4e5a3cd24c 100644 > --- a/arch/arm64/include/asm/cpufeature.h > +++ b/arch/arm64/include/asm/cpufeature.h > @@ -847,8 +847,7 @@ static inline bool system_supports_poe(void) > > static inline bool system_supports_gcs(void) > { > - return IS_ENABLED(CONFIG_ARM64_GCS) && > - alternative_has_cap_unlikely(ARM64_HAS_GCS); > + return alternative_has_cap_unlikely(ARM64_HAS_GCS); > } > > static inline bool system_supports_haft(void) > -- > 2.39.2.101.g768bb238c484.dirty >
On Thu, Dec 05, 2024 at 01:48:10PM +0000, Robin Murphy wrote: > Since system_supports_gcs() ends up referring to cpucap_is_possible(), > teach the latter about GCS for consistency with similar features. Not clear why this is part of a series, there's no obvious relationship with patch 1? > static inline bool system_supports_gcs(void) > { > - return IS_ENABLED(CONFIG_ARM64_GCS) && > - alternative_has_cap_unlikely(ARM64_HAS_GCS); > + return alternative_has_cap_unlikely(ARM64_HAS_GCS); > } Ah, this is bitrot since the series was on the list for so long. As well as HAFT which Rutland mentioned it looks like _bti_kernel(), _irq_prio_masking() and possibly others have the same thing. Ideally we'd have no uses of IS_ENABLED() in these functions so it's a bit more obvious. In any case Reviewed-by: Mark Brown <broonie@kernel.org>
On Thu, Dec 05, 2024 at 03:04:11PM +0000, Mark Brown wrote: > On Thu, Dec 05, 2024 at 01:48:10PM +0000, Robin Murphy wrote: > > Since system_supports_gcs() ends up referring to cpucap_is_possible(), > > teach the latter about GCS for consistency with similar features. > > Not clear why this is part of a series, there's no obvious relationship > with patch 1? No, probably Robing forgot to pass --no-thread to git. > > static inline bool system_supports_gcs(void) > > { > > - return IS_ENABLED(CONFIG_ARM64_GCS) && > > - alternative_has_cap_unlikely(ARM64_HAS_GCS); > > + return alternative_has_cap_unlikely(ARM64_HAS_GCS); > > } > > Ah, this is bitrot since the series was on the list for so long. As > well as HAFT which Rutland mentioned it looks like _bti_kernel(), > _irq_prio_masking() and possibly others have the same thing. Ideally > we'd have no uses of IS_ENABLED() in these functions so it's a bit more > obvious. In any case > > Reviewed-by: Mark Brown <broonie@kernel.org> Thanks. This patch will go in via the arm64 tree. I'll leave the other for the smmu tree.
On Thu, Dec 05, 2024 at 03:04:11PM +0000, Mark Brown wrote: > On Thu, Dec 05, 2024 at 01:48:10PM +0000, Robin Murphy wrote: > > static inline bool system_supports_gcs(void) > > { > > - return IS_ENABLED(CONFIG_ARM64_GCS) && > > - alternative_has_cap_unlikely(ARM64_HAS_GCS); > > + return alternative_has_cap_unlikely(ARM64_HAS_GCS); > > } > > Ah, this is bitrot since the series was on the list for so long. As > well as HAFT which Rutland mentioned it looks like _bti_kernel(), > _irq_prio_masking() and possibly others have the same thing. In <asm/cpufeature.h>, only system_supports_gcs() and system_supports_haft() don't use cpucap_is_possible(); all the other IS_ENABLED() checks are for additional dependent properties, and rely on the base case being handled in cpucap_is_possible(). In <asm/cpucaps.h> we have: static __always_inline bool cpucap_is_possible(const unsigned int cap) { ... switch (cap) { ... case ARM64_HAS_PAN: return IS_ENABLED(CONFIG_ARM64_PAN); ... case ARM64_BTI: return IS_ENABLED(CONFIG_ARM64_BTI); ... case ARM64_HAS_GIC_PRIO_MASKING: return IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI); ... } } In <asm/cpufeature.h> we have: // base case static inline bool system_uses_hw_pan(void) { return alternative_has_cap_unlikely(ARM64_HAS_PAN); } // additional dependent property static inline bool system_uses_ttbr0_pan(void) { return IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) && !system_uses_hw_pan(); } ... // base case static __always_inline bool system_uses_irq_prio_masking(void) { return alternative_has_cap_unlikely(ARM64_HAS_GIC_PRIO_MASKING); } // additional dependent property static inline bool system_has_prio_mask_debugging(void) { return IS_ENABLED(CONFIG_ARM64_DEBUG_PRIORITY_MASKING) && system_uses_irq_prio_masking(); } ... // base case static inline bool system_supports_bti(void) { return cpus_have_final_cap(ARM64_BTI); } // additional dependent property static inline bool system_supports_bti_kernel(void) { return IS_ENABLED(CONFIG_ARM64_BTI_KERNEL) && cpus_have_final_boot_cap(ARM64_BTI); } > Ideally we'd have no uses of IS_ENABLED() in these functions so it's a > bit more obvious. For the three cases above we can't do that without additional cpucaps, and I think that'd be worse. I don't see anything we can practically do other than add comments, and I suspect that's not going to help either. Mark.
On Thu, Dec 05, 2024 at 03:40:57PM +0000, Mark Rutland wrote: > In <asm/cpufeature.h>, only system_supports_gcs() and > system_supports_haft() don't use cpucap_is_possible(); all the other > IS_ENABLED() checks are for additional dependent properties, and rely on > the base case being handled in cpucap_is_possible(). Ah, yes - I got lost in indirections while checking.
On 05/12/2024 3:25 pm, Catalin Marinas wrote: > On Thu, Dec 05, 2024 at 03:04:11PM +0000, Mark Brown wrote: >> On Thu, Dec 05, 2024 at 01:48:10PM +0000, Robin Murphy wrote: >>> Since system_supports_gcs() ends up referring to cpucap_is_possible(), >>> teach the latter about GCS for consistency with similar features. >> >> Not clear why this is part of a series, there's no obvious relationship >> with patch 1? > > No, probably Robing forgot to pass --no-thread to git. Yeah, this was just a tangent from poking around figuring out how to detect GCS being enabled for the main patch. >>> static inline bool system_supports_gcs(void) >>> { >>> - return IS_ENABLED(CONFIG_ARM64_GCS) && >>> - alternative_has_cap_unlikely(ARM64_HAS_GCS); >>> + return alternative_has_cap_unlikely(ARM64_HAS_GCS); >>> } >> >> Ah, this is bitrot since the series was on the list for so long. As >> well as HAFT which Rutland mentioned it looks like _bti_kernel(), >> _irq_prio_masking() and possibly others have the same thing. Ideally >> we'd have no uses of IS_ENABLED() in these functions so it's a bit more >> obvious. In any case >> >> Reviewed-by: Mark Brown <broonie@kernel.org> > > Thanks. This patch will go in via the arm64 tree. I'll leave the other > for the smmu tree. Thanks! Robin.
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h index 201a46efd918..cbbf70e0f204 100644 --- a/arch/arm64/include/asm/cpucaps.h +++ b/arch/arm64/include/asm/cpucaps.h @@ -44,6 +44,8 @@ cpucap_is_possible(const unsigned int cap) return IS_ENABLED(CONFIG_ARM64_TLB_RANGE); case ARM64_HAS_S1POE: return IS_ENABLED(CONFIG_ARM64_POE); + case ARM64_HAS_GCS: + return IS_ENABLED(CONFIG_ARM64_GCS); case ARM64_UNMAP_KERNEL_AT_EL0: return IS_ENABLED(CONFIG_UNMAP_KERNEL_AT_EL0); case ARM64_WORKAROUND_843419: diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index b64e49bd9d10..8b4e5a3cd24c 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -847,8 +847,7 @@ static inline bool system_supports_poe(void) static inline bool system_supports_gcs(void) { - return IS_ENABLED(CONFIG_ARM64_GCS) && - alternative_has_cap_unlikely(ARM64_HAS_GCS); + return alternative_has_cap_unlikely(ARM64_HAS_GCS); } static inline bool system_supports_haft(void)
Since system_supports_gcs() ends up referring to cpucap_is_possible(), teach the latter about GCS for consistency with similar features. Signed-off-by: Robin Murphy <robin.murphy@arm.com> --- arch/arm64/include/asm/cpucaps.h | 2 ++ arch/arm64/include/asm/cpufeature.h | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-)