[PATCH v2] KVM: arm64: Make L1Ip feature in CTR_EL0 writable from userspace

[PATCH v2] KVM: arm64: Make L1Ip feature in CTR_EL0 writable from userspace

[Shameer Kolothum]

Only allow userspace to set VIPT(0b10) or PIPT(0b11) for L1Ip based on
what hardware reports as both AIVIVT (0b01) and VPIPT (0b00) are
documented as reserved.

Using a VIPT for Guest where hardware reports PIPT may lead to over
invalidation, but is still correct. Hence, we can allow downgrading
PIPT to VIPT, but not the other way around.

Reviewed-by: Sebastian Ott <sebott@redhat.com>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
v1: 
https://lore.kernel.org/kvmarm/20241017085925.40532-1-shameerali.kolothum.thodi@huawei.com/

Changes from v1:
 -As per Marc's suggestion modified the way set_ctr_el0() restricted
  the write which implied ordered values for the types. Instead
  explicitly listed  all possible policy types.
 -Added R-by tags(Thanks!).

Thanks,
Shameer
---
 arch/arm64/kvm/sys_regs.c | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index d97ccf1c1558..0a0f336b0edd 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1872,6 +1872,34 @@ static int set_id_aa64pfr1_el1(struct kvm_vcpu *vcpu,
 	return set_id_reg(vcpu, rd, user_val);
 }
 
+static int set_ctr_el0(struct kvm_vcpu *vcpu,
+		       const struct sys_reg_desc *rd, u64 user_val)
+{
+	u8 user_L1Ip = SYS_FIELD_GET(CTR_EL0, L1Ip, user_val);
+
+	/*
+	 * Both AIVIVT (0b01) and VPIPT (0b00) are documented as reserved.
+	 * Hence only allow to set VIPT(0b10) or PIPT(0b11) for L1Ip based
+	 * on what hardware reports.
+	 *
+	 * Using a VIPT software model on PIPT will lead to over invalidation,
+	 * but still correct. Hence, we can allow downgrading PIPT to VIPT,
+	 * but not the other way around. This is handled via arm64_ftr_safe_value()
+	 * as CTR_EL0 ftr_bits has L1Ip field with type FTR_EXACT and safe value
+	 * set as VIPT.
+	 */
+	switch (user_L1Ip) {
+	case CTR_EL0_L1Ip_RESERVED_VPIPT:
+	case CTR_EL0_L1Ip_RESERVED_AIVIVT:
+		return -EINVAL;
+	case CTR_EL0_L1Ip_VIPT:
+	case CTR_EL0_L1Ip_PIPT:
+		return set_id_reg(vcpu, rd, user_val);
+	default:
+		return -ENOENT;
+	}
+}
+
 /*
  * cpufeature ID register user accessors
  *
@@ -2614,10 +2642,12 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
 	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
 	{ SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 },
-	ID_WRITABLE(CTR_EL0, CTR_EL0_DIC_MASK |
-			     CTR_EL0_IDC_MASK |
-			     CTR_EL0_DminLine_MASK |
-			     CTR_EL0_IminLine_MASK),
+	ID_FILTERED(CTR_EL0, ctr_el0,
+		    CTR_EL0_DIC_MASK |
+		    CTR_EL0_IDC_MASK |
+		    CTR_EL0_DminLine_MASK |
+		    CTR_EL0_L1Ip_MASK |
+		    CTR_EL0_IminLine_MASK),
 	{ SYS_DESC(SYS_SVCR), undef_access, reset_val, SVCR, 0, .visibility = sme_visibility  },
 	{ SYS_DESC(SYS_FPMR), undef_access, reset_val, FPMR, 0, .visibility = fp8_visibility },
 
-- 
2.45.2

RE: [PATCH v2] KVM: arm64: Make L1Ip feature in CTR_EL0 writable from userspace

[Shameerali Kolothum Thodi]

Hi Oliver,

A gentle ping on this. I think it still applies cleanly on top of Joey's v6 of MPAM
Hide series.

Thanks,
Shameer

> -----Original Message-----
> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On
> Behalf Of Shameer Kolothum
> Sent: Tuesday, October 22, 2024 8:40 AM
> To: kvmarm@lists.linux.dev; maz@kernel.org; oliver.upton@linux.dev
> Cc: james.morse@arm.com; joey.gouly@arm.com;
> suzuki.poulose@arm.com; sebott@redhat.com; yuzenghui
> <yuzenghui@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> linux-arm-kernel@lists.infradead.org; Linuxarm <linuxarm@huawei.com>
> Subject: [PATCH v2] KVM: arm64: Make L1Ip feature in CTR_EL0 writable
> from userspace
> 
> Only allow userspace to set VIPT(0b10) or PIPT(0b11) for L1Ip based on
> what hardware reports as both AIVIVT (0b01) and VPIPT (0b00) are
> documented as reserved.
> 
> Using a VIPT for Guest where hardware reports PIPT may lead to over
> invalidation, but is still correct. Hence, we can allow downgrading
> PIPT to VIPT, but not the other way around.
> 
> Reviewed-by: Sebastian Ott <sebott@redhat.com>
> Reviewed-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> ---
> v1:
> https://lore.kernel.org/kvmarm/20241017085925.40532-1-
> shameerali.kolothum.thodi@huawei.com/
> 
> Changes from v1:
>  -As per Marc's suggestion modified the way set_ctr_el0() restricted
>   the write which implied ordered values for the types. Instead
>   explicitly listed  all possible policy types.
>  -Added R-by tags(Thanks!).
> 
> Thanks,
> Shameer
> ---
>  arch/arm64/kvm/sys_regs.c | 38 ++++++++++++++++++++++++++++++++++---
> -
>  1 file changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index d97ccf1c1558..0a0f336b0edd 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1872,6 +1872,34 @@ static int set_id_aa64pfr1_el1(struct kvm_vcpu
> *vcpu,
>  	return set_id_reg(vcpu, rd, user_val);
>  }
> 
> +static int set_ctr_el0(struct kvm_vcpu *vcpu,
> +		       const struct sys_reg_desc *rd, u64 user_val)
> +{
> +	u8 user_L1Ip = SYS_FIELD_GET(CTR_EL0, L1Ip, user_val);
> +
> +	/*
> +	 * Both AIVIVT (0b01) and VPIPT (0b00) are documented as reserved.
> +	 * Hence only allow to set VIPT(0b10) or PIPT(0b11) for L1Ip based
> +	 * on what hardware reports.
> +	 *
> +	 * Using a VIPT software model on PIPT will lead to over
> invalidation,
> +	 * but still correct. Hence, we can allow downgrading PIPT to VIPT,
> +	 * but not the other way around. This is handled via
> arm64_ftr_safe_value()
> +	 * as CTR_EL0 ftr_bits has L1Ip field with type FTR_EXACT and safe
> value
> +	 * set as VIPT.
> +	 */
> +	switch (user_L1Ip) {
> +	case CTR_EL0_L1Ip_RESERVED_VPIPT:
> +	case CTR_EL0_L1Ip_RESERVED_AIVIVT:
> +		return -EINVAL;
> +	case CTR_EL0_L1Ip_VIPT:
> +	case CTR_EL0_L1Ip_PIPT:
> +		return set_id_reg(vcpu, rd, user_val);
> +	default:
> +		return -ENOENT;
> +	}
> +}
> +
>  /*
>   * cpufeature ID register user accessors
>   *
> @@ -2614,10 +2642,12 @@ static const struct sys_reg_desc sys_reg_descs[]
> = {
>  	{ SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
>  	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
>  	{ SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown,
> CSSELR_EL1 },
> -	ID_WRITABLE(CTR_EL0, CTR_EL0_DIC_MASK |
> -			     CTR_EL0_IDC_MASK |
> -			     CTR_EL0_DminLine_MASK |
> -			     CTR_EL0_IminLine_MASK),
> +	ID_FILTERED(CTR_EL0, ctr_el0,
> +		    CTR_EL0_DIC_MASK |
> +		    CTR_EL0_IDC_MASK |
> +		    CTR_EL0_DminLine_MASK |
> +		    CTR_EL0_L1Ip_MASK |
> +		    CTR_EL0_IminLine_MASK),
>  	{ SYS_DESC(SYS_SVCR), undef_access, reset_val, SVCR, 0, .visibility =
> sme_visibility  },
>  	{ SYS_DESC(SYS_FPMR), undef_access, reset_val, FPMR, 0, .visibility =
> fp8_visibility },
> 
> --
> 2.45.2
> 

Re: [PATCH v2] KVM: arm64: Make L1Ip feature in CTR_EL0 writable from userspace

[Oliver Upton]

On Tue, 22 Oct 2024 08:39:43 +0100, Shameer Kolothum wrote:
> Only allow userspace to set VIPT(0b10) or PIPT(0b11) for L1Ip based on
> what hardware reports as both AIVIVT (0b01) and VPIPT (0b00) are
> documented as reserved.
> 
> Using a VIPT for Guest where hardware reports PIPT may lead to over
> invalidation, but is still correct. Hence, we can allow downgrading
> PIPT to VIPT, but not the other way around.
> 
> [...]

Sorry I didn't grab this earlier, for some reason I'm not getting your mail
at my linux.dev address.

Applied to kvmarm/next, thanks!

[1/1] KVM: arm64: Make L1Ip feature in CTR_EL0 writable from userspace
      https://git.kernel.org/kvmarm/kvmarm/c/e9b57d7f9740

--
Best,
Oliver