Re: [PATCH 06/12] KVM: arm64: Zero ID_AA64PFR0_EL1.GIC when no GICv3 is presented to the guest

[Oliver Upton <oliver.upton@linux.dev>]

On Tue, Aug 20, 2024 at 11:03:43AM +0100, Marc Zyngier wrote:
> In order to be consistent, we shouldn't advertise a GICv3 when none
> is actually usable by the guest.
> 
> Wipe the feature when these conditions apply, and allow the field
> to be written from userspace.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index bc2d54da3827..7d00d7e359e1 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2365,7 +2365,6 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  		   ID_AA64PFR0_EL1_MPAM |
>  		   ID_AA64PFR0_EL1_SVE |
>  		   ID_AA64PFR0_EL1_RAS |
> -		   ID_AA64PFR0_EL1_GIC |
>  		   ID_AA64PFR0_EL1_AdvSIMD |
>  		   ID_AA64PFR0_EL1_FP), },
>  	ID_SANITISED(ID_AA64PFR1_EL1),
> @@ -4634,6 +4633,11 @@ int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu)
>  
>  	guard(mutex)(&kvm->arch.config_lock);
>  
> +	if (!kvm_has_gicv3(kvm)) {
> +		kvm->arch.id_regs[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] &= ~ID_AA64PFR0_EL1_GIC_MASK;
> +		kvm->arch.id_regs[IDREG_IDX(SYS_ID_PFR1_EL1)] &= ~ID_PFR1_EL1_GIC_MASK;
> +	}
> +

Hmm, should we use the ID register field as the source of truth for
kvm_has_gicv3() at this point?

I think what you have in patch #1 makes good sense for a stable
backport. Using the ID register from this point forward would make the
behavior consistent for a stupid userspace instantiated GICv3 for the VM
but clobbered it from the ID register.

AFAICT all other usage of kvm_has_gicv3() happens after
kvm_finalize_sys_regs(), so it should take this last-minute fixup into
account.

-- 
Thanks,
Oliver