* [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible()
2024-12-05 13:48 [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features Robin Murphy
@ 2024-12-05 13:48 ` Robin Murphy
2024-12-05 14:23 ` Mark Rutland
2024-12-05 15:04 ` Mark Brown
2024-12-05 18:14 ` [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features Jean-Philippe Brucker
` (2 subsequent siblings)
3 siblings, 2 replies; 12+ messages in thread
From: Robin Murphy @ 2024-12-05 13:48 UTC (permalink / raw)
To: will, catalin.marinas
Cc: joro, jean-philippe, iommu, linux-arm-kernel, mark.rutland,
joey.gouly, ryan.roberts, broonie
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(-)
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible()
2024-12-05 13:48 ` [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible() Robin Murphy
@ 2024-12-05 14:23 ` Mark Rutland
2024-12-05 15:04 ` Mark Brown
1 sibling, 0 replies; 12+ messages in thread
From: Mark Rutland @ 2024-12-05 14:23 UTC (permalink / raw)
To: Robin Murphy
Cc: will, catalin.marinas, joro, jean-philippe, iommu,
linux-arm-kernel, joey.gouly, ryan.roberts, broonie
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible()
2024-12-05 13:48 ` [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible() Robin Murphy
2024-12-05 14:23 ` Mark Rutland
@ 2024-12-05 15:04 ` Mark Brown
2024-12-05 15:25 ` Catalin Marinas
2024-12-05 15:40 ` Mark Rutland
1 sibling, 2 replies; 12+ messages in thread
From: Mark Brown @ 2024-12-05 15:04 UTC (permalink / raw)
To: Robin Murphy
Cc: will, catalin.marinas, joro, jean-philippe, iommu,
linux-arm-kernel, mark.rutland, joey.gouly, ryan.roberts
[-- Attachment #1: Type: text/plain, Size: 844 bytes --]
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>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible()
2024-12-05 15:04 ` Mark Brown
@ 2024-12-05 15:25 ` Catalin Marinas
2024-12-05 15:55 ` Robin Murphy
2024-12-05 15:40 ` Mark Rutland
1 sibling, 1 reply; 12+ messages in thread
From: Catalin Marinas @ 2024-12-05 15:25 UTC (permalink / raw)
To: Mark Brown
Cc: Robin Murphy, will, joro, jean-philippe, iommu, linux-arm-kernel,
mark.rutland, joey.gouly, ryan.roberts
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.
--
Catalin
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible()
2024-12-05 15:25 ` Catalin Marinas
@ 2024-12-05 15:55 ` Robin Murphy
0 siblings, 0 replies; 12+ messages in thread
From: Robin Murphy @ 2024-12-05 15:55 UTC (permalink / raw)
To: Catalin Marinas, Mark Brown
Cc: will, joro, jean-philippe, iommu, linux-arm-kernel, mark.rutland,
joey.gouly, ryan.roberts
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible()
2024-12-05 15:04 ` Mark Brown
2024-12-05 15:25 ` Catalin Marinas
@ 2024-12-05 15:40 ` Mark Rutland
2024-12-05 15:52 ` Mark Brown
1 sibling, 1 reply; 12+ messages in thread
From: Mark Rutland @ 2024-12-05 15:40 UTC (permalink / raw)
To: Mark Brown
Cc: Robin Murphy, will, catalin.marinas, joro, jean-philippe, iommu,
linux-arm-kernel, joey.gouly, ryan.roberts
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.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible()
2024-12-05 15:40 ` Mark Rutland
@ 2024-12-05 15:52 ` Mark Brown
0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2024-12-05 15:52 UTC (permalink / raw)
To: Mark Rutland
Cc: Robin Murphy, will, catalin.marinas, joro, jean-philippe, iommu,
linux-arm-kernel, joey.gouly, ryan.roberts
[-- Attachment #1: Type: text/plain, Size: 384 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features
2024-12-05 13:48 [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features Robin Murphy
2024-12-05 13:48 ` [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible() Robin Murphy
@ 2024-12-05 18:14 ` Jean-Philippe Brucker
2024-12-06 12:03 ` Robin Murphy
2024-12-05 18:23 ` (subset) " Catalin Marinas
2024-12-10 0:17 ` Will Deacon
3 siblings, 1 reply; 12+ messages in thread
From: Jean-Philippe Brucker @ 2024-12-05 18:14 UTC (permalink / raw)
To: Robin Murphy
Cc: will, catalin.marinas, joro, iommu, linux-arm-kernel,
mark.rutland, joey.gouly, ryan.roberts, broonie
On Thu, Dec 05, 2024 at 01:48:09PM +0000, Robin Murphy wrote:
> Process pagetables may now be using new permission-indirection-based
> features which an SMMU may not understand when given such a table for
> SVA. Although SMMUv3.4 does add its own S1PIE feature, realistically
> we're still going to have to cope with feature mismatches between CPUs
> and SMMUs, so let's start simple and essentially just document the
> expectations for what falls out as-is. Although it seems unlikely for
> SVA applications to also depend on memory-hardening features, or
> vice-versa,
Hard to predict, but SVA could be indirectly enabled by some common
library: for example a libcrypto plugin that shares the address space with
an accelerator when one is available (see uadk_engine). I'm not familiar
with the POE use-cases, but I guess a common library could also enable
those memory-hardening features, such that unsuspecting applications have
everything enabled.
> the relative lifecycles make it tricky to enforce mutual
> exclusivity. Thankfully our PIE index allocation makes it relatively
> benign for an SMMU to keep interpreting them as direct permissions, the
> only real implication is that an SVA application cannot harden itself
> against its own devices with these features. Thus, inform the user about
> that just in case they have other expectations.
>
> Also we don't (yet) support LPA2, so deny SVA entirely if we're going to
> misunderstand the pagetable format altogether.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> index 1d3e71569775..9ba596430e7c 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> @@ -112,6 +112,15 @@ void arm_smmu_make_sva_cd(struct arm_smmu_cd *target,
> * from the current CPU register
> */
> target->data[3] = cpu_to_le64(read_sysreg(mair_el1));
> +
> + /*
> + * Note that we don't bother with S1PIE on the SMMU, we just rely on
> + * our default encoding scheme matching direct permissions anyway.
> + * SMMU has no notion of S1POE nor GCS, so make sure that is clear if
> + * either is enabled for CPUs, just in case anyone imagines otherwise.
> + */
> + if (system_supports_poe() || system_supports_gcs())
> + dev_warn_once(master->smmu->dev, "SVA devices ignore permission overlays and GCS\n");
If POE tightens the page permissions, enabling SVA on top lets the
application easily bypass the POE protection. Could we actually return
false in arm_smmu_sva_supported() instead of displaying a warning that
likely no one will notice?
I guess enforcing !(SVA & POE) on an address space would be too much work?
Thanks,
Jean
> }
> EXPORT_SYMBOL_IF_KUNIT(arm_smmu_make_sva_cd);
>
> @@ -206,8 +215,12 @@ bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
> unsigned long asid_bits;
> u32 feat_mask = ARM_SMMU_FEAT_COHERENCY;
>
> - if (vabits_actual == 52)
> + if (vabits_actual == 52) {
> + /* We don't support LPA2 */
> + if (PAGE_SIZE != SZ_64K)
> + return false;
> feat_mask |= ARM_SMMU_FEAT_VAX;
> + }
>
> if ((smmu->features & feat_mask) != feat_mask)
> return false;
> --
> 2.39.2.101.g768bb238c484.dirty
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features
2024-12-05 18:14 ` [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features Jean-Philippe Brucker
@ 2024-12-06 12:03 ` Robin Murphy
0 siblings, 0 replies; 12+ messages in thread
From: Robin Murphy @ 2024-12-06 12:03 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: will, catalin.marinas, joro, iommu, linux-arm-kernel,
mark.rutland, joey.gouly, ryan.roberts, broonie
On 2024-12-05 6:14 pm, Jean-Philippe Brucker wrote:
> On Thu, Dec 05, 2024 at 01:48:09PM +0000, Robin Murphy wrote:
>> Process pagetables may now be using new permission-indirection-based
>> features which an SMMU may not understand when given such a table for
>> SVA. Although SMMUv3.4 does add its own S1PIE feature, realistically
>> we're still going to have to cope with feature mismatches between CPUs
>> and SMMUs, so let's start simple and essentially just document the
>> expectations for what falls out as-is. Although it seems unlikely for
>> SVA applications to also depend on memory-hardening features, or
>> vice-versa,
>
> Hard to predict, but SVA could be indirectly enabled by some common
> library: for example a libcrypto plugin that shares the address space with
> an accelerator when one is available (see uadk_engine). I'm not familiar
> with the POE use-cases, but I guess a common library could also enable
> those memory-hardening features, such that unsuspecting applications have
> everything enabled.
>
>> the relative lifecycles make it tricky to enforce mutual
>> exclusivity. Thankfully our PIE index allocation makes it relatively
>> benign for an SMMU to keep interpreting them as direct permissions, the
>> only real implication is that an SVA application cannot harden itself
>> against its own devices with these features. Thus, inform the user about
>> that just in case they have other expectations.
>>
>> Also we don't (yet) support LPA2, so deny SVA entirely if we're going to
>> misunderstand the pagetable format altogether.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
>> index 1d3e71569775..9ba596430e7c 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
>> @@ -112,6 +112,15 @@ void arm_smmu_make_sva_cd(struct arm_smmu_cd *target,
>> * from the current CPU register
>> */
>> target->data[3] = cpu_to_le64(read_sysreg(mair_el1));
>> +
>> + /*
>> + * Note that we don't bother with S1PIE on the SMMU, we just rely on
>> + * our default encoding scheme matching direct permissions anyway.
>> + * SMMU has no notion of S1POE nor GCS, so make sure that is clear if
>> + * either is enabled for CPUs, just in case anyone imagines otherwise.
>> + */
>> + if (system_supports_poe() || system_supports_gcs())
>> + dev_warn_once(master->smmu->dev, "SVA devices ignore permission overlays and GCS\n");
>
> If POE tightens the page permissions, enabling SVA on top lets the
> application easily bypass the POE protection. Could we actually return
> false in arm_smmu_sva_supported() instead of displaying a warning that
> likely no one will notice?
The difficulty with both features is that they're always unconditionally
*enabled* if present, but the SVA concern only arises if and when they
are *actively used*, and that can be a transient thing. It doesn't seem
reasonable to effectively make SVA unusable on newer CPUs (or at least
force users to boot with MMFR overrides to lose GCS/POE if they want to
use SVA), especially when it's far from clear how bothered anyone's
really going to be about this anyway. I just think we should do at least
*something* to remember and acknowledge that certain CPU features impact
SVA too.
> I guess enforcing !(SVA & POE) on an address space would be too much work?
Yes, to attempt to do this "properly" I believe we'd need to keep track
of how many SVA attachments, overlays, and GCS pages (if the SMMU can't
support S1PIE) a process has at any given point in time, then check
those in all 3 places to deny individual usage calls if the "opposing"
feature is currently in use.
Thanks,
Robin.
>
> Thanks,
> Jean
>
>> }
>> EXPORT_SYMBOL_IF_KUNIT(arm_smmu_make_sva_cd);
>>
>> @@ -206,8 +215,12 @@ bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
>> unsigned long asid_bits;
>> u32 feat_mask = ARM_SMMU_FEAT_COHERENCY;
>>
>> - if (vabits_actual == 52)
>> + if (vabits_actual == 52) {
>> + /* We don't support LPA2 */
>> + if (PAGE_SIZE != SZ_64K)
>> + return false;
>> feat_mask |= ARM_SMMU_FEAT_VAX;
>> + }
>>
>> if ((smmu->features & feat_mask) != feat_mask)
>> return false;
>> --
>> 2.39.2.101.g768bb238c484.dirty
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: (subset) [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features
2024-12-05 13:48 [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features Robin Murphy
2024-12-05 13:48 ` [PATCH 2/2] arm64: cpufeature: Add GCS to cpucap_is_possible() Robin Murphy
2024-12-05 18:14 ` [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features Jean-Philippe Brucker
@ 2024-12-05 18:23 ` Catalin Marinas
2024-12-10 0:17 ` Will Deacon
3 siblings, 0 replies; 12+ messages in thread
From: Catalin Marinas @ 2024-12-05 18:23 UTC (permalink / raw)
To: will, Robin Murphy
Cc: joro, jean-philippe, iommu, linux-arm-kernel, mark.rutland,
joey.gouly, ryan.roberts, broonie
On Thu, 05 Dec 2024 13:48:09 +0000, Robin Murphy wrote:
> Process pagetables may now be using new permission-indirection-based
> features which an SMMU may not understand when given such a table for
> SVA. Although SMMUv3.4 does add its own S1PIE feature, realistically
> we're still going to have to cope with feature mismatches between CPUs
> and SMMUs, so let's start simple and essentially just document the
> expectations for what falls out as-is. Although it seems unlikely for
> SVA applications to also depend on memory-hardening features, or
> vice-versa, the relative lifecycles make it tricky to enforce mutual
> exclusivity. Thankfully our PIE index allocation makes it relatively
> benign for an SMMU to keep interpreting them as direct permissions, the
> only real implication is that an SVA application cannot harden itself
> against its own devices with these features. Thus, inform the user about
> that just in case they have other expectations.
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[2/2] arm64: cpufeature: Add GCS to cpucap_is_possible()
https://git.kernel.org/arm64/c/f00b53f1614f
--
Catalin
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features
2024-12-05 13:48 [PATCH 1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features Robin Murphy
` (2 preceding siblings ...)
2024-12-05 18:23 ` (subset) " Catalin Marinas
@ 2024-12-10 0:17 ` Will Deacon
3 siblings, 0 replies; 12+ messages in thread
From: Will Deacon @ 2024-12-10 0:17 UTC (permalink / raw)
To: catalin.marinas, Robin Murphy
Cc: kernel-team, Will Deacon, joro, jean-philippe, iommu,
linux-arm-kernel, mark.rutland, joey.gouly, ryan.roberts, broonie
On Thu, 05 Dec 2024 13:48:09 +0000, Robin Murphy wrote:
> Process pagetables may now be using new permission-indirection-based
> features which an SMMU may not understand when given such a table for
> SVA. Although SMMUv3.4 does add its own S1PIE feature, realistically
> we're still going to have to cope with feature mismatches between CPUs
> and SMMUs, so let's start simple and essentially just document the
> expectations for what falls out as-is. Although it seems unlikely for
> SVA applications to also depend on memory-hardening features, or
> vice-versa, the relative lifecycles make it tricky to enforce mutual
> exclusivity. Thankfully our PIE index allocation makes it relatively
> benign for an SMMU to keep interpreting them as direct permissions, the
> only real implication is that an SVA application cannot harden itself
> against its own devices with these features. Thus, inform the user about
> that just in case they have other expectations.
>
> [...]
Applied SMMU change to will (for-joerg/arm-smmu/updates), thanks!
[1/2] iommu/arm-smmu-v3: Document SVA interaction with new pagetable features
https://git.kernel.org/will/c/6e192214c6c8
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
^ permalink raw reply [flat|nested] 12+ messages in thread