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 <[email protected]>
> ---
> 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 <[email protected]>
Thanks!
Leo
