Re: [PATCH v15 2/6] KVM: arm64: Manage GCS access and registers for guests

[Marc Zyngier <maz@kernel.org>]

On Wed, 20 Aug 2025 15:14:42 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> GCS introduces a number of system registers for EL1 and EL0, on systems

and EL2.

> with GCS we need to context switch them and expose them to VMMs to allow
> guests to use GCS.
> 
> In order to allow guests to use GCS we also need to configure
> HCRX_EL2.GCSEn, if this is not set GCS instructions will be noops and
> CHKFEAT will report GCS as disabled.  Also enable fine grained traps for
> access to the GCS registers by guests which do not have the feature
> enabled.

I don't see any FGT configuration in this patch. As far as I can tell,
the FGU generation already takes care of that particular case.

> 
> In order to allow userspace to control availability of the feature to
> guests we enable writability for only ID_AA64PFR1_EL1.GCS, this is a
> deliberately conservative choice to avoid errors due to oversights.
> Further fields should be made writable in future.

I'm not sure what you mean by that. Making the feature field writable
is only allowable if we have some level of support (and otherwise we
should prevent both the feature being exposed, and the field being
writable).

So future fields being writable will only happen when the features are
fully supported, and only then.

Please clarify, or drop this altogether.

>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_emulate.h       |  3 +++
>  arch/arm64/include/asm/kvm_host.h          | 14 ++++++++++++++
>  arch/arm64/include/asm/vncr_mapping.h      |  2 ++
>  arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 31 ++++++++++++++++++++++++++++++
>  arch/arm64/kvm/hyp/vhe/sysreg-sr.c         | 10 ++++++++++
>  arch/arm64/kvm/sys_regs.c                  | 31 +++++++++++++++++++++++++++++-
>  6 files changed, 90 insertions(+), 1 deletion(-)
>

[...]

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 82ffb3b3b3cf..592cb5d6497a 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -138,6 +138,8 @@ static bool get_el2_to_el1_mapping(unsigned int reg,
>  		MAPPED_EL2_SYSREG(PIR_EL2,     PIR_EL1,     NULL	     );
>  		MAPPED_EL2_SYSREG(PIRE0_EL2,   PIRE0_EL1,   NULL	     );
>  		MAPPED_EL2_SYSREG(POR_EL2,     POR_EL1,     NULL	     );
> +		MAPPED_EL2_SYSREG(GCSCR_EL2,   GCSCR_EL1,   NULL             );
> +		MAPPED_EL2_SYSREG(GCSPR_EL2,   GCSPR_EL1,   NULL             );

How is the state accessed when loaded on the CPU? You seem to be
missing accessors for these two registers, affecting both EL1 and EL2
in the guest.

	M.

-- 
Jazz isn't dead. It just smells funny.