Re: [PATCH v20 09/14] KVM: arm64: Allow GCS to be enabled for guests

[Leonardo Bras <leo.bras@arm.com>]

On Tue, Sep 01, 2026 at 10:47:07PM +0100, Mark Brown wrote:
> Now that required functionality for GCS is in place expose
> ID_AA64PFR1_EL1.GCS, allowing guests to be given the feature.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/kvm/nested.c   | 3 +--
>  arch/arm64/kvm/sys_regs.c | 5 +++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 17123f0b6dab..a9dea85b6192 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1702,11 +1702,10 @@ u64 limit_nv_id_reg(struct kvm *kvm, u32 reg, u64 val)
>  		break;
>  
>  	case SYS_ID_AA64PFR1_EL1:
> -		/* Only support BTI, SSBS, CSV2_frac */
> +		/* Only support GCS, BTI, SSBS, CSV2_frac */
>  		val &= ~(ID_AA64PFR1_EL1_PFAR		|
>  			 ID_AA64PFR1_EL1_MTEX		|
>  			 ID_AA64PFR1_EL1_THE		|
> -			 ID_AA64PFR1_EL1_GCS		|
>  			 ID_AA64PFR1_EL1_MTE_frac	|
>  			 ID_AA64PFR1_EL1_NMI		|
>  			 ID_AA64PFR1_EL1_SME		|

So, IIUC, this is used to filter features for guests that believe to have 
an EL2, and now this allows GCS to be used by them.

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 1f80c26a9839..fb2c9fd42e24 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2193,10 +2193,12 @@ static u64 sanitise_id_aa64pfr1_el1(const struct kvm_vcpu *vcpu, u64 val)
>  	      SYS_FIELD_GET(ID_AA64PFR0_EL1, RAS, pfr0) == ID_AA64PFR0_EL1_RAS_IMP))
>  		val &= ~ID_AA64PFR1_EL1_RAS_frac;
>  
> +	if (!system_supports_gcs())
> +		val &= ~ID_AA64PFR1_EL1_GCS;
> +
>  	val &= ~ID_AA64PFR1_EL1_SME;
>  	val &= ~ID_AA64PFR1_EL1_RNDR_trap;
>  	val &= ~ID_AA64PFR1_EL1_NMI;
> -	val &= ~ID_AA64PFR1_EL1_GCS;
>  	val &= ~ID_AA64PFR1_EL1_THE;
>  	val &= ~ID_AA64PFR1_EL1_MTEX;
>  	val &= ~ID_AA64PFR1_EL1_PFAR;

And here it says to only mask the feature bit if it's not available.

IIUC this sanitize function is called when resetting a vcpu, is that right?


> @@ -3444,7 +3446,6 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  				     ~(ID_AA64PFR1_EL1_PFAR |
>  				       ID_AA64PFR1_EL1_MTEX |
>  				       ID_AA64PFR1_EL1_THE |
> -				       ID_AA64PFR1_EL1_GCS |
>  				       ID_AA64PFR1_EL1_MTE_frac |
>  				       ID_AA64PFR1_EL1_NMI |
>  				       ID_AA64PFR1_EL1_RNDR_trap |
> 

This one looks like to be a mask that's used to compare with 
arm64_ftr_regs[] on arm64_check_features(). That seems to be called on 
set_id_aa64pfr1_el1() which is used when we get an ioclt to set a system 
register.

So, basically, does it use this mask to check if the set value is valid?

(So, 1 for nested, 1 for reset, and one for user trying to set a value?)

If I did not miss anything, FWIW:
Reviewed-by: Leonardo Bras <leo.bras@arm.com>

Thanks!
Leo